Skip to content

Commit 59558b1

Browse files
authored
Respect UV_HTTP_RETRIES in uv publish (#15106)
Previously, publish would always use the default retries, now it respects `UV_HTTP_RETRIES` Some awkward error handling to avoid pulling anyhow into uv-publish.
1 parent aa758ae commit 59558b1

File tree

4 files changed

+40
-21
lines changed

4 files changed

+40
-21
lines changed

crates/uv-client/src/base_client.rs

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use std::error::Error;
22
use std::fmt::Debug;
33
use std::fmt::Write;
4+
use std::num::ParseIntError;
45
use std::path::Path;
56
use std::sync::Arc;
67
use std::time::Duration;
78
use std::{env, io, iter};
89

9-
use anyhow::Context;
1010
use anyhow::anyhow;
1111
use http::{
1212
HeaderMap, HeaderName, HeaderValue, Method, StatusCode,
@@ -22,6 +22,7 @@ use reqwest_retry::policies::ExponentialBackoff;
2222
use reqwest_retry::{
2323
DefaultRetryableStrategy, RetryTransientMiddleware, Retryable, RetryableStrategy,
2424
};
25+
use thiserror::Error;
2526
use tracing::{debug, trace};
2627
use url::ParseError;
2728
use url::Url;
@@ -42,7 +43,9 @@ use crate::linehaul::LineHaul;
4243
use crate::middleware::OfflineMiddleware;
4344
use crate::tls::read_identity;
4445

46+
/// Do not use this value directly outside tests, use [`retries_from_env`] instead.
4547
pub const DEFAULT_RETRIES: u32 = 3;
48+
4649
/// Maximum number of redirects to follow before giving up.
4750
///
4851
/// This is the default used by [`reqwest`].
@@ -169,23 +172,13 @@ impl<'a> BaseClientBuilder<'a> {
169172
self
170173
}
171174

172-
/// Read the retry count from [`EnvVars::UV_HTTP_RETRIES`] if set, otherwise, make no change.
175+
/// Read the retry count from [`EnvVars::UV_HTTP_RETRIES`] if set, otherwise use the default
176+
/// retries.
173177
///
174178
/// Errors when [`EnvVars::UV_HTTP_RETRIES`] is not a valid u32.
175-
pub fn retries_from_env(self) -> anyhow::Result<Self> {
176-
// TODO(zanieb): We should probably parse this in another layer, but there's not a natural
177-
// fit for it right now
178-
if let Some(value) = env::var_os(EnvVars::UV_HTTP_RETRIES) {
179-
Ok(self.retries(
180-
value
181-
.to_string_lossy()
182-
.as_ref()
183-
.parse::<u32>()
184-
.context("Failed to parse `UV_HTTP_RETRIES`")?,
185-
))
186-
} else {
187-
Ok(self)
188-
}
179+
pub fn retries_from_env(mut self) -> Result<Self, RetryParsingError> {
180+
self.retries = retries_from_env()?;
181+
Ok(self)
189182
}
190183

191184
#[must_use]
@@ -982,6 +975,26 @@ fn find_sources<E: Error + 'static>(orig: &dyn Error) -> impl Iterator<Item = &E
982975
iter::successors(find_source::<E>(orig), |&err| find_source(err))
983976
}
984977

978+
// TODO(konsti): Remove once we find a native home for `retries_from_env`
979+
#[derive(Debug, Error)]
980+
pub enum RetryParsingError {
981+
#[error("Failed to parse `UV_HTTP_RETRIES`")]
982+
ParseInt(#[from] ParseIntError),
983+
}
984+
985+
/// Read the retry count from [`EnvVars::UV_HTTP_RETRIES`] if set, otherwise, make no change.
986+
///
987+
/// Errors when [`EnvVars::UV_HTTP_RETRIES`] is not a valid u32.
988+
pub fn retries_from_env() -> Result<u32, RetryParsingError> {
989+
// TODO(zanieb): We should probably parse this in another layer, but there's not a natural
990+
// fit for it right now
991+
if let Some(value) = env::var_os(EnvVars::UV_HTTP_RETRIES) {
992+
Ok(value.to_string_lossy().as_ref().parse::<u32>()?)
993+
} else {
994+
Ok(DEFAULT_RETRIES)
995+
}
996+
}
997+
985998
#[cfg(test)]
986999
mod tests {
9871000
use super::*;

crates/uv-client/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
pub use base_client::{
22
AuthIntegration, BaseClient, BaseClientBuilder, DEFAULT_RETRIES, ExtraMiddleware,
3-
RedirectClientWithMiddleware, RequestBuilder, UvRetryableStrategy, is_extended_transient_error,
3+
RedirectClientWithMiddleware, RequestBuilder, RetryParsingError, UvRetryableStrategy,
4+
is_extended_transient_error, retries_from_env,
45
};
56
pub use cached_client::{CacheControl, CachedClient, CachedClientError, DataWithCachePolicy};
67
pub use error::{Error, ErrorKind, WrappedReqwestError};

crates/uv-publish/src/lib.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ use url::Url;
2727
use uv_auth::Credentials;
2828
use uv_cache::{Cache, Refresh};
2929
use uv_client::{
30-
BaseClient, DEFAULT_RETRIES, MetadataFormat, OwnedArchive, RegistryClientBuilder,
31-
RequestBuilder, UvRetryableStrategy,
30+
BaseClient, MetadataFormat, OwnedArchive, RegistryClientBuilder, RequestBuilder,
31+
RetryParsingError, UvRetryableStrategy, retries_from_env,
3232
};
3333
use uv_configuration::{KeyringProviderType, TrustedPublishing};
3434
use uv_distribution_filename::{DistFilename, SourceDistExtension, SourceDistFilename};
@@ -78,6 +78,8 @@ pub enum PublishError {
7878
},
7979
#[error("Hash is missing in index for {0}")]
8080
MissingHash(Box<DistFilename>),
81+
#[error(transparent)]
82+
RetryParsing(#[from] RetryParsingError),
8183
}
8284

8385
/// Failure to get the metadata for a specific file.
@@ -397,7 +399,7 @@ pub async fn upload(
397399
let mut n_past_retries = 0;
398400
let start_time = SystemTime::now();
399401
// N.B. We cannot use the client policy here because it is set to zero retries
400-
let retry_policy = ExponentialBackoff::builder().build_with_max_retries(DEFAULT_RETRIES);
402+
let retry_policy = ExponentialBackoff::builder().build_with_max_retries(retries_from_env()?);
401403
loop {
402404
let (request, idx) = build_request(
403405
file,

crates/uv/src/commands/project/mod.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,9 @@ pub(crate) enum ProjectError {
262262
#[error(transparent)]
263263
Io(#[from] std::io::Error),
264264

265+
#[error(transparent)]
266+
RetryParsing(#[from] uv_client::RetryParsingError),
267+
265268
#[error(transparent)]
266269
Anyhow(#[from] anyhow::Error),
267270
}
@@ -1708,7 +1711,7 @@ pub(crate) async fn resolve_names(
17081711

17091712
let client_builder = BaseClientBuilder::new()
17101713
.retries_from_env()
1711-
.map_err(uv_requirements::Error::ClientError)?
1714+
.map_err(|err| uv_requirements::Error::ClientError(err.into()))?
17121715
.connectivity(network_settings.connectivity)
17131716
.native_tls(network_settings.native_tls)
17141717
.keyring(*keyring_provider)

0 commit comments

Comments
 (0)