Bug #10448

[CWL] --disable-reuse not communicated to cwl-runner

Added by Peter Amstutz about 3 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
11/03/2016
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-

Description

When the --disable-reuse flag is provided on the command line with --submit, the --disable-reuse option is not propagated to the job submitted to run cwl-runner on the pipeline.

Suggested solution is to add "arv:reuse" to script parameters and ensure it is handled up by the crunch script running the job.


Subtasks

Task #10452: Review 10448-cwl-runner-disable-reuseResolvedTom Clegg

Associated revisions

Revision 98fcb678
Added by Peter Amstutz about 3 years ago

Merge branch '10448-cwl-runner-disable-reuse' closes #10448

History

#1 Updated by Peter Amstutz about 3 years ago

  • Description updated (diff)

#2 Updated by Peter Amstutz about 3 years ago

  • Status changed from New to In Progress
  • Assigned To set to Peter Amstutz

#3 Updated by Tom Clegg about 3 years ago

Tests are failing:

IOError: [Errno 2] No such file or directory: 'tests/discovery-f850a12e2daa64bc0c800e6d680f8e07.json'

I'm guessing this is a file that didn't get git-added.

But before you add it, why not get the real discovery doc, and modify it as needed? Including a [modified] snapshot of a real discovery doc from some point in history (I'm just guessing that's what's in the missing file) seems like an unnecessary maintenance headache.

Suggest renaming _rootDesc.py to something a little more normal/explanatory like mock_discovery.py.

The actual enable_reuse / use_existing changes LGTM.

#4 Updated by Peter Amstutz about 3 years ago

Tom Clegg wrote:

Tests are failing:

[...]

I'm guessing this is a file that didn't get git-added.

You're right. Fixed.

But before you add it, why not get the real discovery doc, and modify it as needed?

Including a [modified] snapshot of a real discovery doc from some point in history (I'm just guessing that's what's in the missing file) seems like an unnecessary maintenance headache.

It is a real (unmodified) discovery doc, which is modified on the fly for the one test that needs it. Since all tests are supposed to use mocks instead of relying on a live API server, I don't want to require having an API server running for no other purpose than to serve a discovery doc.

Suggest renaming _rootDesc.py to something a little more normal/explanatory like mock_discovery.py.

Done.

The actual enable_reuse / use_existing changes LGTM.

I added a couple more tests to assert that the flag is correctly propagated through.

Now at 661cc25829ee78ef42dc4602648ae015a93c12e7

#5 Updated by Tom Clegg about 3 years ago

Please back out the discovery document snapshot.

Let's fix the bug for now, and leave the test infrastructure as it was until we come up with a better solution.

#6 Updated by Peter Amstutz about 3 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:98fcb678a39f72945a9c947315beb31178bd4fc3.

#7 Updated by Peter Amstutz about 3 years ago

Fixed in package python-arvados-cwl-runner 1.0.20161103172804-3

Also available in: Atom PDF