Move parsing http retries to EnvironmentOptions#16284
Move parsing http retries to EnvironmentOptions#16284konstin merged 12 commits intoastral-sh:mainfrom
Conversation
|
|
||
| /// Create a [`RetryPolicy`] for the client. | ||
| fn retry_policy(&self) -> ExponentialBackoff { | ||
| pub fn retry_policy(&self) -> ExponentialBackoff { |
There was a problem hiding this comment.
I made this function public and use this
Just not to use DEFAULT_RETRIES
| .or(http_timeout) | ||
| .unwrap_or(Duration::from_secs(15 * 60)), | ||
| http_timeout: http_timeout.unwrap_or(Duration::from_secs(30)), | ||
| http_retries: parse_u32_environment_variable(EnvVars::UV_HTTP_RETRIES)?.unwrap_or(3), |
There was a problem hiding this comment.
I used 3 here
It should be the same as DEFAULT_RETRIES
I used 3 instead of DEFAULT_RETRIES because I do not want to add uv-client as deps here
There was a problem hiding this comment.
There is already a transitive dependency from uv-settings to uv-client, so adding the dependency is effectively for free.
5f21ead to
9be8dfa
Compare
|
|
||
| /// Parse a integer environment variable. | ||
| fn parse_u32_environment_variable(name: &'static str) -> Result<Option<u32>, Error> { | ||
| parse_integer_environment_variable(name, "expected an non-negative integer") |
There was a problem hiding this comment.
Can we show the actual error message from parsing instead of our generic one? This solves the error message regression.
After doing that, we shouldn't need those wrapper functions anymore, but can use e.g. parse_integer_environment_variable::<u32> directly.
| error: Failed to parse `UV_HTTP_RETRIES` | ||
| Caused by: number too large to fit in target type | ||
| error: Failed to parse environment variable `UV_HTTP_RETRIES` with invalid value `999999999999`: expected an non-negative integer |
There was a problem hiding this comment.
This is the regression in the error message I mentioned in the other comment.
Head branch was pushed to by a user without write access
|
Could you please retry this? |
|
Our depot runner are affected by the AWS outage (https://status.depot.dev/), sorry for the disruption. |
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
Summary
UV_HTTP_RETRIEStoEnvironmentOptionsRelates #14720
Test Plan