-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add UV_HTTP_RETRIES to customize retry counts
#14544
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ use std::sync::Arc; | |
| use std::time::Duration; | ||
| use std::{env, io, iter}; | ||
|
|
||
| use anyhow::Context; | ||
| use anyhow::anyhow; | ||
| use http::{ | ||
| HeaderMap, HeaderName, HeaderValue, Method, StatusCode, | ||
|
|
@@ -166,6 +167,25 @@ impl<'a> BaseClientBuilder<'a> { | |
| self | ||
| } | ||
|
|
||
| /// Read the retry count from [`EnvVars::UV_HTTP_RETRIES`] if set, otherwise, make no change. | ||
| /// | ||
| /// Errors when [`EnvVars::UV_HTTP_RETRIES`] is not a valid u32. | ||
| pub fn retries_from_env(self) -> anyhow::Result<Self> { | ||
| // TODO(zanieb): We should probably parse this in another layer, but there's not a natural | ||
| // fit for it right now | ||
| if let Some(value) = env::var_os(EnvVars::UV_HTTP_RETRIES) { | ||
| Ok(self.retries( | ||
| value | ||
| .to_string_lossy() | ||
| .as_ref() | ||
| .parse::<u32>() | ||
| .context("Failed to parse `UV_HTTP_RETRIES`")?, | ||
| )) | ||
| } else { | ||
| Ok(self) | ||
| } | ||
| } | ||
|
|
||
| #[must_use] | ||
| pub fn native_tls(mut self, native_tls: bool) -> Self { | ||
| self.native_tls = native_tls; | ||
|
|
@@ -238,7 +258,11 @@ impl<'a> BaseClientBuilder<'a> { | |
|
|
||
| /// Create a [`RetryPolicy`] for the client. | ||
| fn retry_policy(&self) -> ExponentialBackoff { | ||
| ExponentialBackoff::builder().build_with_max_retries(self.retries) | ||
| let mut builder = ExponentialBackoff::builder(); | ||
| if env::var_os(EnvVars::UV_TEST_NO_HTTP_RETRY_DELAY).is_some() { | ||
| builder = builder.retry_bounds(Duration::from_millis(0), Duration::from_millis(0)); | ||
| } | ||
|
Comment on lines
+262
to
+264
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need this so a test that we consume all the retries isn't slow. |
||
| builder.build_with_max_retries(self.retries) | ||
| } | ||
|
|
||
| pub fn build(&self) -> BaseClient { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -193,6 +193,9 @@ async fn venv_impl( | |
| .unwrap_or(PathBuf::from(".venv")), | ||
| ); | ||
|
|
||
| // TODO(zanieb): We don't use [`BaseClientBuilder::retries_from_env`] here because it's a pain | ||
| // to map into a miette diagnostic. We should just remove miette diagnostics here, we're not | ||
| // using them elsewhere. | ||
| let client_builder = BaseClientBuilder::default() | ||
|
Comment on lines
+196
to
199
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I gave up here, it looked annoying and we only need the client for fetching seed packages.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing miette in #14546 |
||
| .connectivity(network_settings.connectivity) | ||
| .native_tls(network_settings.native_tls) | ||
|
|
||
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.
Can probably be
to_strsince if it's non-UTF-8 it won't parse, right?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.
Then I need to write an error case for when that's returns
None, it seemed easier to just let it hit the parse error.