Feature #19070

--create/update-workflow should take owner into account uploading dependencies

Added by Peter Amstutz 28 days ago. Updated 6 days ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
CWL
Target version:
Start date:
05/04/2022
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Subtasks

Task #19096: Review 19070-update-workflow-depsResolvedPeter Amstutz

Task #19134: Fix integration test on jenkins ResolvedPeter Amstutz


Related issues

Related to Arvados Epics - Story #17848: Improve a-c-r usabilityIn Progress07/01/202105/31/2022

History

#1 Updated by Peter Amstutz 28 days ago

  • Status changed from New to In Progress

#2 Updated by Peter Amstutz 28 days ago

  • Assigned To set to Peter Amstutz
  • Category set to CWL
  • Tracker changed from Bug to Feature

#3 Updated by Peter Amstutz 28 days ago

  • Subject changed from --update-workflow should take owner into account uploading dependencies to --create/update-workflow should take owner into account uploading dependencies

#4 Updated by Peter Amstutz 21 days ago

#5 Updated by Peter Amstutz 21 days ago

19070-update-workflow-deps @ bea50608be40f5ecf218fa5be75213579ad95eca

  • add --copy-deps option, when enabled, dependencies of the workflow (input collections and docker images) are copied into the destination project
  • copy-deps is default behavior for --create-workflow and --update-workflow
  • when executing workflows from command line, does not copy anything by default (can still provide --copy-deps)
  • added integration test which tries the various options and checks the results

developer-run-tests: #3086

#6 Updated by Peter Amstutz 19 days ago

19070-update-workflow-deps @ 1eb881796a672247db251fd7298b0a7253f83259

  • Everything from above, but also make --match-submitter-images default behavior for --create-workflow and --update-workflow

developer-run-tests: #3096

#8 Updated by Lucas Di Pentima 15 days ago

Some questions & comments:

  • In file sdk/python/arvados/commands/keepdocker.py:L410 Is there an advantage of uploading the docker image as a first attempt instead of using one that's already in Arvados?
  • Typo 'depenencies' at --no-copy-deps's help message
  • In file sdk/cwl/arvados_cwl/executor.py: L547 refers to runtimeContext.update_workflow but L554 refers to self.runtimeContext.update_worflow, how both context differentiate between each other? the one passed to arv_executor() should have porecedence?
  • This behavior change may need some documentation updates and probably a migration note, in case clients rely on the old way of doing things, wdyt?

#9 Updated by Peter Amstutz 14 days ago

  • Target version changed from 2022-05-11 sprint to 2022-05-25 sprint

#10 Updated by Peter Amstutz 13 days ago

Lucas Di Pentima wrote:

Some questions & comments:

  • In file sdk/python/arvados/commands/keepdocker.py:L410 Is there an advantage of uploading the docker image as a first attempt instead of using one that's already in Arvados?

Because if the image exists in Docker locally, the assumption is that we want to match the image id of the local copy with the Arvados copy, which means we have to find out what the local version is.

As a fallback behavior, if you don't have the image or don't have Docker, you can still use arv-keepdocker to copy an image from one project to another.

  • Typo 'depenencies' at --no-copy-deps's help message

Fixed.

  • In file sdk/cwl/arvados_cwl/executor.py: L547 refers to runtimeContext.update_workflow but L554 refers to self.runtimeContext.update_worflow, how both context differentiate between each other? the one passed to arv_executor() should have porecedence?

The code was being sloppy, some places referred to the runtimeContext that was passed in, and other places referred to the runtimeContext from arvRunner (because it wasn't passing in runtimeContext). I cleaned this up, now it uses the passed-in runtimeContext almost everywhere. The only reason I didn't do it the first time is because it was extra work (but I went ahead with it since you called it out).

  • This behavior change may need some documentation updates and probably a migration note, in case clients rely on the old way of doing things, wdyt?

I updated the documentation.

19070-update-workflow-deps @ 45e2ea82d68e74774bf61f6966e3777973e61845

developer-run-tests: #3118

#11 Updated by Lucas Di Pentima 12 days ago

This LGTM, thanks!

#12 Updated by Peter Amstutz 12 days ago

  • Release set to 51

#13 Updated by Peter Amstutz 6 days ago

  • Status changed from In Progress to Resolved

Also available in: Atom PDF