Bug #10110

arv-copy to support Workflow

Added by Tom Morris over 4 years ago. Updated over 4 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
-
Target version:
Start date:
01/04/2017
Due date:
% Done:

100%

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

Description

Usage:

$ arv-copy --src source_cluster --dst dest_cluster 962eh-7fd4e-660fw86nz0w77mk
  1. Copy "name", "description" and "definition" fields.
  2. Load the contents of the "definiton" field as YAML. Recursively search the YAML for "location" fields, for example
    location: 'keep:2463fa9efeb75e099685528b3b9071e0+438/19.fasta.bwt'
    Parse the keep: URI to get the PDH the collection and add it to a set.
  3. Go through the set and use existing arv-copy functionality to copy the collections referenced in the definition.

Subtasks

Task #10824: Review 10110-arv-copy-workflowResolvedLucas Di Pentima

Associated revisions

Revision 565e1cc8
Added by Radhika Chippada over 4 years ago

closes #10110
Merge branch '10110-arv-copy-workflow'

History

#1 Updated by Tom Morris over 4 years ago

  • Target version set to Arvados Future Sprints
  • Story points set to 2.0

Manually tested. Update documentation as well.

#2 Updated by Radhika Chippada over 4 years ago

  • Assigned To set to Radhika Chippada
  • Target version changed from Arvados Future Sprints to 2017-01-18 sprint

#3 Updated by Peter Amstutz over 4 years ago

  • Description updated (diff)
  • Target version changed from 2017-01-18 sprint to Arvados Future Sprints

#4 Updated by Peter Amstutz over 4 years ago

  • Target version changed from Arvados Future Sprints to 2017-01-18 sprint

#5 Updated by Peter Amstutz over 4 years ago

  • Description updated (diff)

#6 Updated by Radhika Chippada over 4 years ago

  • Status changed from New to In Progress

#7 Updated by Lucas Di Pentima over 4 years ago

Some comments:

  • sdk/python/arvados/commands/arv_copy.py file:
    • Line 116: Command’s description could be updated to reflect the new feature
    • Lines 412-422: Should that documentation be as a docstring inside the func?
    • Line 438: Should wf_def be passed instead of graph?
    • Lines 458-460: To improve readability, do you think it would be better to replace those 3 lines with a single assignment like: var = one() or another() ?
    • Line 468: I believe that if can be replaced with elif
  • As you mentioned, I too think that ruamel.yaml should be added as a dependency. It seems that Python doesn’t have ALL the batteries included after all.
  • Do you think a test for this feature would be desirable?

#8 Updated by Radhika Chippada over 4 years ago

Line 116: Command’s description could be updated to reflect the new feature

You are right. Updated.

Lines 412-422: Should that documentation be as a docstring inside the func?

The documentation for recursive, project_uuid etc already exists. This comment is additional code comment only, aimed at code maintenance.

Line 438: Should wf_def be passed instead of graph?

Thanks for catching this. Corrected.

Lines 458-460: To improve readability, do you think it would be better to replace those 3 lines with a single assignment like: var = one() or another() ?

Updated and thanks for teaching :)

Line 468: I believe that if can be replaced with elif

Corrected

As you mentioned, I too think that ruamel.yaml should be added as a dependency. It seems that Python doesn’t have ALL the batteries included after all.

Added ruamel.yaml to setup.py per Peter's suggestion

Do you think a test for this feature would be desirable?

Unfortunately, this feature has no automated tests yet and Peter agreed that I only manually test for now.

Thanks.

#9 Updated by Lucas Di Pentima over 4 years ago

LGTM. Thanks!

#10 Updated by Radhika Chippada over 4 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:565e1cc82ed0af5a4d58ea78d6e0294afdd1cba8.

Also available in: Atom PDF