Project

General

Profile

Actions
Copy link

Bug #10110

closed

arv-copy to support Workflow

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

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
-
Target version:
2017-01-18 sprint
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 1 (0 open — 1 closed)

Task #10824: Review 10110-arv-copy-workflowResolvedLucas Di Pentima01/04/2017Actions
Actions
Copy link
 #1

Updated by Tom Morris over 7 years ago

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

Manually tested. Update documentation as well.

Actions
Copy link
 #2

Updated by Radhika Chippada over 7 years ago

  • Assigned To set to Radhika Chippada
  • Target version changed from Arvados Future Sprints to 2017-01-18 sprint
Actions
Copy link
 #3

Updated by Peter Amstutz over 7 years ago

  • Description updated (diff)
  • Target version changed from 2017-01-18 sprint to Arvados Future Sprints
Actions
Copy link
 #4

Updated by Peter Amstutz over 7 years ago

  • Target version changed from Arvados Future Sprints to 2017-01-18 sprint
Actions
Copy link
 #5

Updated by Peter Amstutz over 7 years ago

  • Description updated (diff)
Actions
Copy link
 #6

Updated by Radhika Chippada over 7 years ago

  • Status changed from New to In Progress
Actions
Copy link
 #7

Updated by Lucas Di Pentima over 7 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?
Actions
Copy link
 #8

Updated by Radhika Chippada over 7 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.

Actions
Copy link
 #9

Updated by Lucas Di Pentima over 7 years ago

LGTM. Thanks!

Actions
Copy link
 #10

Updated by Radhika Chippada over 7 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:565e1cc82ed0af5a4d58ea78d6e0294afdd1cba8.

Actions
Copy link

Also available in: Atom PDF