-
Notifications
You must be signed in to change notification settings - Fork 27
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
Do fail (raise Exception, should be non-0 exit) download if any download fails #1536
base: master
Are you sure you want to change the base?
Conversation
…tempts_allowed no functionality changes should be done. Also removed some stale TODO comments -- DownloadDirectory already downloads into a file with ".dandidownload" instead of original filename. And we do report progress. ETA should be estimated outside IMHO.
Otherwise it is hard-to-impossible to script using "dandi download" reliably
if resuming: | ||
lgr.debug("%s - resumed download. Need to check full checksum.", path) | ||
else: | ||
assert not downloaded_digest |
Check warning
Code scanning / CodeQL
Unreachable code Warning
def _check_if_more_attempts_allowed( | ||
path: Path, | ||
exc: requests.RequestException, | ||
attempt: int, | ||
attempts_allowed: int, | ||
downloaded_in_attempt: int, | ||
) -> int | None: |
Check notice
Code scanning / CodeQL
Explicit returns mixed with implicit (fall through) returns Note
# if is_access_denied(exc) or attempt >= 2: | ||
# raise |
Check notice
Code scanning / CodeQL
Commented-out code Note
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1536 +/- ##
==========================================
+ Coverage 88.44% 88.55% +0.10%
==========================================
Files 78 78
Lines 10691 10721 +30
==========================================
+ Hits 9456 9494 +38
+ Misses 1235 1227 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Sits on top of #1535