Bug #6657
closed
[SDKs] arv-copy has an unhelpful error message when `git` command is not available
Added by Joshua Randall almost 9 years ago.
Updated almost 9 years ago.
Assigned To:
Radhika Chippada
Description
When arv-copy is asked to copy a pipeline template recursively to a --dst-git-repo, it requires the `git` command in order to run. When the `git` command is not available, my expectation would be that I might get an error something like:
- arv-copy --src qr1hi --dst 7lnae --project-uuid 7lnae-j7d0g-9r429yt0rcrdl8x --dst-git-repo jr17/test qr1hi-p5p6p-8584ylktoumirn0
Error: git command is not available, please ensure it is available in the PATH
What actually happens is the following stack trace:
- arv-copy --src qr1hi --dst 7lnae --project-uuid 7lnae-j7d0g-9r429yt0rcrdl8x --dst-git-repo jr17/test qr1hi-p5p6p-8584ylktoumirn0
Traceback (most recent call last):
File "/usr/local/bin/arv-copy", line 4, in <module>
main()
File "/usr/local/lib/python2.7/dist-packages/arvados/commands/arv_copy.py", line 128, in main
src_arv, dst_arv, args)
File "/usr/local/lib/python2.7/dist-packages/arvados/commands/arv_copy.py", line 272, in copy_pipeline_template
copy_git_repos(pt, src, dst, args.dst_git_repo, args)
File "/usr/local/lib/python2.7/dist-packages/arvados/commands/arv_copy.py", line 371, in copy_git_repos
migrate_jobspec(component, src, dst, dst_repo, args)
File "/usr/local/lib/python2.7/dist-packages/arvados/commands/arv_copy.py", line 344, in migrate_jobspec
copy_git_repo(repo, src, dst, dst_repo, script_version, args)
File "/usr/local/lib/python2.7/dist-packages/arvados/commands/arv_copy.py", line 617, in copy_git_repo
cwd=os.path.dirname(local_repo_dir[src_git_repo]))
File "/usr/local/lib/python2.7/dist-packages/arvados/util.py", line 45, in run_command
p = subprocess.Popen(execargs, **kwargs)
File "/usr/lib/python2.7/subprocess.py", line 679, in init
errread, errwrite)
File "/usr/lib/python2.7/subprocess.py", line 1249, in _execute_child
raise child_exception
OSError: [Errno 2] No such file or directory
- Category set to SDKs
- Target version set to 2015-08-05 sprint
- Story points set to 0.5
We should implement this exactly as specified. arv-copy should check for git up front so it can report this error right away any time it is asked to perform an operation that requires copying git repositories, before it copies anything.
- Assigned To set to Peter Amstutz
- Assigned To changed from Peter Amstutz to Radhika Chippada
- Subject changed from arv-copy has an unhelpful error message when `git` command is not available to [SDKs] arv-copy has an unhelpful error message when `git` command is not available
- Status changed from New to In Progress
- The check for git needs to happen in
copy_pipeline_instance() before it does anything else. It should go before the check if not args.dst_git_repo:
- Instead of raising an exception, it should use
abort
- In the documentation, we should avoid advising the user to install a package using
apt-get because they may be using a different platform, such as Red Hat. I think saying "you must have git installed" is sufficient.
Updated accordingly. Thanks.
Peter Amstutz wrote:
- The check for git needs to happen in
copy_pipeline_instance() before it does anything else. It should go before the check if not args.dst_git_repo:
Don't we also need it in copy_pipeline_template? Like I said in note-1, this check should be done before any copy operation that requires access to a git repository.
Updated both copy_pipeline_instance and copy_pipeline_template methods.
Peter said:
The check for git needs to happen in copy_pipeline_instance
I implemented this suggestion (updated copy_pipeline_instance and copy_pipeline_template methods). However, I am not sure if this is the right place to check for git version? It appears that there are other places where the copy_git_repo is invoked (for example: migrate_jobspec). Should I move this check back into the copy_git_repo method?
Thanks.
Radhika Chippada wrote:
Peter said:
The check for git needs to happen in copy_pipeline_instance
I implemented this suggestion (updated copy_pipeline_instance and copy_pipeline_template methods). However, I am not sure if this is the right place to check for git version? It appears that there are other places where the copy_git_repo is invoked (for example: migrate_jobspec). Should I move this check back into the copy_git_repo method?
I don't think that will meet the second half of the functional requirements in note-1. If you do that, arv-copy may copy other objects the pipeline uses before it tries to copy the git commits and fails. This will waste the user's time and clutter their project, which is undesirable.
migrate_jobspec only gets called from copy_git_repos, which only gets called from copy_pipeline_instance and copy_pipeline_template. So checking at the top of the pipeline functions should be sufficient.
Brett said:
migrate_jobspec only gets called from copy_git_repos, which only gets called from copy_pipeline_instance and copy_pipeline_template. So checking at the top of the pipeline functions should be sufficient
Yes, just checked that it is only executed in this context. So, we should be all set. Thanks.
- Status changed from In Progress to Resolved
- % Done changed from 0 to 100
Applied in changeset arvados|commit:ccf30f40f46ea450d7ab3766f0923b486a7450d3.
Also available in: Atom
PDF
Updated by 
Updated by 
Updated by Radhika Chippada