Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BF: download-url: Widen exception around add_url_to_file call #4817

Merged
merged 1 commit into from Aug 12, 2020

Conversation

kyleam
Copy link
Collaborator

@kyleam kyleam commented Aug 11, 2020

6c59334 (RF: Avoid batch mode for a single file in download-url,
2019-12-10) made the add_url_to_file() call use batch=False if only
one URL is given. In this case, a failed addurl call will result in a
CommandError rather than a AnnexBatchCommandError, which is derived
from CommandError.

6c59334 (RF: Avoid batch mode for a single file in download-url,
2019-12-10) made the add_url_to_file() call use batch=False if only
one URL is given.  In this case, a failed addurl call will result in a
CommandError rather than a AnnexBatchCommandError, which is derived
from CommandError.
@codecov
Copy link

codecov bot commented Aug 11, 2020

Codecov Report

Merging #4817 into maint will increase coverage by 0.15%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##            maint    #4817      +/-   ##
==========================================
+ Coverage   89.46%   89.62%   +0.15%     
==========================================
  Files         288      288              
  Lines       40351    40351              
==========================================
+ Hits        36101    36163      +62     
+ Misses       4250     4188      -62     
Impacted Files Coverage Δ
datalad/interface/download_url.py 97.91% <66.66%> (ø)
datalad/downloaders/http.py 81.85% <0.00%> (-2.71%) ⬇️
datalad/downloaders/tests/test_http.py 87.71% <0.00%> (-2.22%) ⬇️
datalad/support/gitrepo.py 90.32% <0.00%> (-0.08%) ⬇️
datalad/utils.py 81.92% <0.00%> (+0.09%) ⬆️
datalad/support/tests/test_annexrepo.py 96.74% <0.00%> (+0.15%) ⬆️
datalad/distribution/tests/test_create_sibling.py 99.27% <0.00%> (+0.24%) ⬆️
datalad/tests/test_cmd.py 97.48% <0.00%> (+0.50%) ⬆️
datalad/ui/progressbars.py 77.19% <0.00%> (+0.58%) ⬆️
datalad/core/local/create.py 94.44% <0.00%> (+0.69%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b302a7e...68f8ca5. Read the comment docs.

@yarikoptic
Copy link
Member

yarikoptic commented Aug 12, 2020

makes sense and tests like it -- merging. But I am wondering if there is any specific issue this is addressing?

@yarikoptic yarikoptic merged commit 58cd9bf into datalad:maint Aug 12, 2020
3 of 4 checks passed
@kyleam kyleam deleted the download-url-widen-exception branch Aug 12, 2020
@kyleam
Copy link
Collaborator Author

kyleam commented Aug 12, 2020

But I am wondering if there is any specific issue this is addressing?

I think the issue described is quite specific: when one URL is passed and the git annex addurl call fails, datalad crashes instead of the intended behavior of catching the exception and displaying a warning.

So I think you're asking how I triggered/noticed this issue. When testing the shub downloader, I was patching the base url. That override will of course be seen by the downloader but not the special remote. As a result, the download succeeds, but registering the URL with git annex addurl fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants