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

Use a higher timeout for publishing #7923

Merged
merged 1 commit into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions crates/uv-client/src/base_client.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use std::error::Error;
use std::fmt::Debug;
use std::path::Path;
use std::{env, iter};

use itertools::Itertools;
use reqwest::{Client, ClientBuilder, Response};
use reqwest_middleware::ClientWithMiddleware;
use reqwest_retry::policies::ExponentialBackoff;
use reqwest_retry::{
DefaultRetryableStrategy, RetryTransientMiddleware, Retryable, RetryableStrategy,
};
use std::error::Error;
use std::fmt::Debug;
use std::path::Path;
use std::time::Duration;
use std::{env, iter};
use tracing::debug;
use url::Url;
use uv_auth::AuthMiddleware;
Expand Down Expand Up @@ -52,6 +52,7 @@ pub struct BaseClientBuilder<'a> {
markers: Option<&'a MarkerEnvironment>,
platform: Option<&'a Platform>,
auth_integration: AuthIntegration,
default_timeout: Duration,
}

impl Default for BaseClientBuilder<'_> {
Expand All @@ -72,6 +73,7 @@ impl BaseClientBuilder<'_> {
markers: None,
platform: None,
auth_integration: AuthIntegration::default(),
default_timeout: Duration::from_secs(30),
}
}
}
Expand Down Expand Up @@ -131,6 +133,12 @@ impl<'a> BaseClientBuilder<'a> {
self
}

#[must_use]
pub fn default_timeout(mut self, default_timeout: Duration) -> Self {
self.default_timeout = default_timeout;
self
}

pub fn is_offline(&self) -> bool {
matches!(self.connectivity, Connectivity::Offline)
}
Expand Down Expand Up @@ -161,20 +169,20 @@ impl<'a> BaseClientBuilder<'a> {

// Timeout options, matching https://doc.rust-lang.org/nightly/cargo/reference/config.html#httptimeout
// `UV_REQUEST_TIMEOUT` is provided for backwards compatibility with v0.1.6
let default_timeout = 30;
let timeout = env::var("UV_HTTP_TIMEOUT")
.or_else(|_| env::var("UV_REQUEST_TIMEOUT"))
.or_else(|_| env::var("HTTP_TIMEOUT"))
.and_then(|value| {
value.parse::<u64>()
.map(Duration::from_secs)
.or_else(|_| {
// On parse error, warn and use the default timeout
warn_user_once!("Ignoring invalid value from environment for `UV_HTTP_TIMEOUT`. Expected an integer number of seconds, got \"{value}\".");
Ok(default_timeout)
Ok(self.default_timeout)
})
})
.unwrap_or(default_timeout);
debug!("Using request timeout of {timeout}s");
.unwrap_or(self.default_timeout);
debug!("Using request timeout of {}s", timeout.as_secs());

// Create a secure client that validates certificates.
let raw_client = self.create_client(
Expand Down Expand Up @@ -227,7 +235,7 @@ impl<'a> BaseClientBuilder<'a> {
fn create_client(
&self,
user_agent: &str,
timeout: u64,
timeout: Duration,
ssl_cert_file_exists: bool,
security: Security,
) -> Client {
Expand All @@ -236,7 +244,7 @@ impl<'a> BaseClientBuilder<'a> {
.http1_title_case_headers()
.user_agent(user_agent)
.pool_max_idle_per_host(20)
.read_timeout(std::time::Duration::from_secs(timeout))
.read_timeout(timeout)
.tls_built_in_root_certs(false);

// If necessary, accept invalid certificates.
Expand Down Expand Up @@ -327,7 +335,7 @@ pub struct BaseClient {
/// The connectivity mode to use.
connectivity: Connectivity,
/// Configured client timeout, in seconds.
timeout: u64,
timeout: Duration,
/// Hosts that are trusted to use the insecure client.
allow_insecure_host: Vec<TrustedHost>,
}
Expand Down Expand Up @@ -365,7 +373,7 @@ impl BaseClient {
}

/// The configured client timeout, in seconds.
pub fn timeout(&self) -> u64 {
pub fn timeout(&self) -> Duration {
self.timeout
}

Expand Down
16 changes: 8 additions & 8 deletions crates/uv-client/src/registry_client.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use std::collections::BTreeMap;
use std::fmt::Debug;
use std::path::PathBuf;
use std::str::FromStr;

use async_http_range_reader::AsyncHttpRangeReader;
use futures::{FutureExt, TryStreamExt};
use http::HeaderMap;
use reqwest::{Client, Response, StatusCode};
use reqwest_middleware::ClientWithMiddleware;
use std::collections::BTreeMap;
use std::fmt::Debug;
use std::path::PathBuf;
use std::str::FromStr;
use std::time::Duration;
use tracing::{info_span, instrument, trace, warn, Instrument};
use url::Url;

Expand Down Expand Up @@ -171,7 +171,7 @@ pub struct RegistryClient {
/// The connectivity mode to use.
connectivity: Connectivity,
/// Configured client timeout, in seconds.
timeout: u64,
timeout: Duration,
}

impl RegistryClient {
Expand All @@ -191,7 +191,7 @@ impl RegistryClient {
}

/// Return the timeout this client is configured with, in seconds.
pub fn timeout(&self) -> u64 {
pub fn timeout(&self) -> Duration {
self.timeout
}

Expand Down Expand Up @@ -713,7 +713,7 @@ impl RegistryClient {
std::io::Error::new(
std::io::ErrorKind::TimedOut,
format!(
"Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", self.timeout()
"Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", self.timeout().as_secs()
),
)
} else {
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-distribution/src/distribution_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
io::Error::new(
io::ErrorKind::TimedOut,
format!(
"Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", self.client.unmanaged.timeout()
"Failed to download distribution due to network timeout. Try increasing UV_HTTP_TIMEOUT (current value: {}s).", self.client.unmanaged.timeout().as_secs()
),
)
} else {
Expand Down
4 changes: 4 additions & 0 deletions crates/uv/src/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use anyhow::{bail, Result};
use owo_colors::OwoColorize;
use std::fmt::Write;
use std::sync::Arc;
use std::time::Duration;
use tracing::info;
use url::Url;
use uv_client::{AuthIntegration, BaseClientBuilder, Connectivity, DEFAULT_RETRIES};
Expand Down Expand Up @@ -50,6 +51,9 @@ pub(crate) async fn publish(
.allow_insecure_host(allow_insecure_host)
// Don't try cloning the request to make an unauthenticated request first.
.auth_integration(AuthIntegration::OnlyAuthenticated)
// Set a very high timeout for uploads, connections are often 10x slower on upload than
// download. 15 min is taken from the time a trusted publishing token is valid.
.default_timeout(Duration::from_secs(15 * 60))
.build();
let oidc_client = BaseClientBuilder::new()
.auth_integration(AuthIntegration::NoAuthMiddleware)
Expand Down
Loading