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

Download retry #15

Closed
pothos opened this issue Oct 10, 2023 · 2 comments · Fixed by #42
Closed

Download retry #15

pothos opened this issue Oct 10, 2023 · 2 comments · Fixed by #42
Labels
kind/feature A feature request.

Comments

@pothos
Copy link
Member

pothos commented Oct 10, 2023

  • HTTP requests should time out and be retried on timeouts or HTTP errors (503 etc.) - DNS retries are out of scope, similar to how curl is used already: Integration of ue-rs with update-engine post-install script Flatcar#1028 (comment)
  • Not a blocker, because we could run a bash loop around it but it's not nice because we don't know if it's a download or verification error, and would wastefully retry on verification errors then
@pothos
Copy link
Member Author

pothos commented Nov 27, 2023

Until then, to distinguish download from verification errors we can also do the following: Download into a .download folder and only if finished, move to the .unverified folder. This way the caller can check that downloading worked by looking if the file exists in the .unverified folder.

@pothos
Copy link
Member Author

pothos commented Nov 30, 2023

I think we should also set a connection timeout in the reqwest client builder:
https://docs.rs/reqwest/latest/reqwest/struct.ClientBuilder.html#method.connect_timeout
https://docs.rs/reqwest/latest/reqwest/struct.ClientBuilder.html#method.timeout (can be a problem on slow connections, but with a very large timeout it should be ok and help to eventually make progress)
Note that it needs the tokio timer feature.

In curl we have this:

--retry-delay 1 --retry 60 --retry-connrefused --retry-max-time 60 --connect-timeout 20

In words, we need a retry on temporary http errors, http cancelations, refused connection, and TCP connect timeouts. The read timeout in addition is nice to have when chosen very large.

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

Successfully merging a pull request may close this issue.

2 participants