-
Notifications
You must be signed in to change notification settings - Fork 893
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
Kinnison/retry downloads #2121
Kinnison/retry downloads #2121
Conversation
Previously we would fail to finalise the download tracker on error. This ensures that we allow the download tracker to finish *before* we then propagate any errors. Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
Since we could get 416 if the partial file is corrupted for whatever reason, treat it as a non-client error and therefore we might retry it. Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
This is to be used when the downloader decides it has to retry a download (for example, if it timed out, the socket died, etc) Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
In order that we can cope with 416 and other fixable errors when downloading components, retry the download a few times. Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
fd6b89d
to
e1a5fca
Compare
@payload Thanks for the review, is there anything else you wanted to query? |
When downloading a file, rustup will retry the download a number of times. The | ||
default is 3 times, but if this variable is set to a valid usize then it is the | ||
max retry count. A value of `0` means no retries, thus the default of `3` will | ||
mean a download is tried a total of four times before failing out. |
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.
Hm, shouldn't this go to Readme instead?
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.
My hope is that noone will need to tweak this, if we decide to expose it to users then it becomes a thing we have to support long term, rather than something where in an issue we can say "Please try this and see if it helps" to help decide if we need to change the defaults.
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.
Ahhh...
I think a valid use-case might be increasing the limit for CI (where networking can get unstable). Cargo has a knob for it in .cargo/config
, [net] retries = 92
.
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.
CI is a place where people get even more angry if I change things. Humans seem more prepared to change their own workflows than to change CI setups which "have always worked".
In an attempt to close #1722 as a way to fix #1667, fix #1243, and to fix #2071 more fully, add support for cleanly retrying downloads a fixed number of times.
Also this contains a small cleanup which means we finalise the download tracker properly on errors