-
Notifications
You must be signed in to change notification settings - Fork 1k
Preserve partial downloads on network failure #4695
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
Conversation
|
@FranciscoTGouveia Looks easy enough! I'll give it a test drive when I'm back with my machine :) |
rami3l
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for investigating into this!
OTOH I do think we need to be more careful about filtering the right kind of error since we do want to redownload if the cache is proven to be corrupted for example 🙏
c01dd03 to
235dabb
Compare
235dabb to
1b5abc3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo a minor wording change and some nits, nice work :)
When you are done, just squash the first 3 commits into one (they include some back-and-forth) and we are good to go.
3f4542b to
cb889fe
Compare
cb889fe to
17a4b01
Compare
rami3l
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work :)
Closes #4448.
If
resume_from_partialis set to true, it short-circuits, ensuring theremove_file(path)is not called.