Bug #11948
closedFix failing CWL conformance tests
Added by Peter Amstutz over 7 years ago. Updated over 7 years ago.
Description
New CWL conformance tests were introduced which fail on Arvados.
Fix arvados-cwl-runner to pass these tests.
Updated by Peter Amstutz over 7 years ago
- Description updated (diff)
- Status changed from New to In Progress
- Assigned To set to Peter Amstutz
Updated by Peter Amstutz over 7 years ago
11948-cwl-conformance-fix @ d1a7f9691ff2b9d77106f953c714b1455b228c64
The main change in this branch is to accommodate a new CWL conformance test. This test checks that when a parameter has a default value that is a File or Directory, the default File is not required to exist provided some other File is assigned for the input parameter.
The implementation approach is to scan the document for "default" blocks, and check for Files under "default", and then check if those files exist. If the files do not exist, it deletes the "default" block (so that downstream processing will require a value for that parameter) and removes it from the list of files to try and upload.
Updated by Lucas Di Pentima over 7 years ago
Some questions:
- File sdk/cwl/arvados_cwl/runner.py
- Line 53: Could that
if
be changed to anelif
, or there’s some case when a variable is alist
and also adict
? - Lines 123-128: If I understand it correctly, is an in-place list modification, and I think could be written like this, what do you think?
sc[:] = [x for x in sc if x[“location”] != f[“location”]]
- Line 116: Why is
remove
a list and not simply a boolean var?
- Line 53: Could that
Updated by Peter Amstutz over 7 years ago
Lucas Di Pentima wrote:
Some questions:
- File sdk/cwl/arvados_cwl/runner.py
- Line 53: Could that
if
be changed to anelif
, or there’s some case when a variable is alist
and also adict
?
You're right. Fixed.
- Lines 123-128: If I understand it correctly, is an in-place list modification, and I think could be written like this, what do you think?
That's a good one. Fixed.
- Line 116: Why is
remove
a list and not simply a boolean var?
Python scopes are a little weird. When you define a function inside a function, you can access variables in the outer scope, but assignment only modifies the local scope. So setting "removed = True" wouldn't change the value of the outer "removed" variable. Instead, I use a list to add a level of indirection (sort of like a pointer).
Now @ 2feec8cbac7ef28a47f3c3a9d071b070ef38cb6e
Unit testing¶
arvbox start test --only sdk/cwl
Conformance tests¶
To set up, create a virtualenv on your host, install the CWL SDKs from the branch, start arvbox, then build a arvados/job Docker image:
virtualenv venv && . venv/bin/activate cd ~/arvados/sdk/cwl && python setup.py install arvbox restart dev export ARVADOS_API_HOST= export ARVADOS_API_TOKEN= cd ~/arvados/build && WORKSPACE=$HOME/arvados ./build-dev-docker-jobs-image.sh
Now you can clone the repository with the CWL tests and run them:
git clone https://github.com/common-workflow-language/common-workflow-language.git cd common-workflow-language ./run_test.sh RUNNER=arvados-cwl-runner EXTRA="--api=containers --compute-checksum --disable-reuse"
Updated by Lucas Di Pentima over 7 years ago
Local unit test worked ok on my end.
Updates LGTM.
Conformance tests couldn't be run because I wasn't able to start arvbox, just letting you know instead of blocking you because of my problem with arvbox.
Updated by Tom Morris over 7 years ago
- Target version changed from 2017-07-19 sprint to 2017-08-02 sprint
Updated by Tom Clegg over 7 years ago
- Status changed from In Progress to Resolved