Project

General

Profile

Actions

Bug #12904

closed

arvados-cwl-runner in --local mode allows ECMAScript 6 syntax

Added by Joshua Randall about 8 years ago. Updated over 7 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
-
Category:
Crunch
Target version:
-
Story points:
-

Description

I thought that CWL specified ECMAScript 5.1 syntax (in strict mode). However, we accidentally used the ECMAScript 6 `for...of` syntax (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of) in an ECMAScript function body and this worked without issue when using `arvados-cwl-runner --local` whereas it correctly gave a SyntaxError when using `arvados-cwl-runner` without `--local`.

I don't think there should be any difference in ECMAScript evaluation when running locally vs not.

Actions #1

Updated by Joshua Randall about 8 years ago

I also note that `console.log()` calls that were accidentally left in the code did not throw an error in `--local` mode, while they did when not running with `--local`.

Actions #2

Updated by Tom Clegg about 8 years ago

It looks like the discrepancy is caused by the arvados/jobs image (which is how JS runs on the cluster) having a different version of nodejs than the node:slim image (which is how JS runs in local mode). One way to ensure they're equal might be for arvados-cwl-runner to use arvados/jobs instead of node:slim when evaluating JS in a docker container.

Either way, it seems like a bug that cwltool is invoking node:slim in such a way that (with the current node:slim) it runs as ECMAScript 6.

Actions #3

Updated by Joshua Randall about 8 years ago

Indeed - `node:slim` is currenty providing nodejs v9.3.0, whereas the arvados/jobs image has v0.10.29

mercury@arvados-shell-ncucu:~/checkouts/arvados-pipelines/cwl$ docker run node:slim nodejs --version
v9.3.0
mercury@arvados-shell-ncucu:~/checkouts/arvados-pipelines/cwl$ docker run arvados/jobs:dc78526ba494973df7d298825e20503353e92adf nodejs --version
v0.10.29

The CWL spec only talks about ECMAScript 5.1 in strict mode, so I'd like to see the nodejs version pinned to one which only provides ECMAScript 5.1 language features and nothing from ES6.

Possibly CWL's InlineJavascriptRequirement should be extended such that the docker image to use for NodeJS can be specified to ensure consistent evaluation behaviour?

Actions #4

Updated by Peter Amstutz almost 8 years ago

arvados-cwl-runner now includes Thomas Hickman's jshint feature from cwltool, so this should be fixed, could you check?

Actions #5

Updated by Peter Amstutz over 7 years ago

  • Status changed from New to Resolved

Assuming resolved.

Actions

Also available in: Atom PDF