Move parsing concurrency env variable to EnvironmentOptions (#16223)

## Summary
- Move parsing `UV_CONCURRENT_INSTALLS`, `UV_CONCURRENT_BUILDS` and
`UV_CONCURRENT_DOWNLOADS` to `EnvironmentOptions`

Relates https://github.com/astral-sh/uv/issues/14720

## Test Plan

- Tests with existing tests
- Add test with invalid parsing
This commit is contained in:
Andrei Berenda 2025-10-13 05:22:30 +04:00 committed by GitHub
parent d536d3f27d
commit c7d3d549e2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 85 additions and 29 deletions

View File

@ -1,3 +1,4 @@
use std::num::NonZeroUsize;
use std::ops::Deref;
use std::path::{Path, PathBuf};
use std::time::Duration;
@ -565,6 +566,13 @@ pub enum Error {
},
}
#[derive(Copy, Clone, Debug)]
pub struct Concurrency {
pub downloads: Option<NonZeroUsize>,
pub builds: Option<NonZeroUsize>,
pub installs: Option<NonZeroUsize>,
}
/// Options loaded from environment variables.
///
/// This is currently a subset of all respected environment variables, most are parsed via Clap at
@ -578,6 +586,7 @@ pub struct EnvironmentOptions {
pub log_context: Option<bool>,
pub http_timeout: Duration,
pub upload_http_timeout: Duration,
pub concurrency: Concurrency,
#[cfg(feature = "tracing-durations-export")]
pub tracing_durations_file: Option<PathBuf>,
}
@ -587,11 +596,9 @@ impl EnvironmentOptions {
pub fn new() -> Result<Self, Error> {
// 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 http_timeout = parse_integer_environment_variable(EnvVars::UV_HTTP_TIMEOUT)?
.or(parse_integer_environment_variable(
EnvVars::UV_REQUEST_TIMEOUT,
)?)
.or(parse_integer_environment_variable(EnvVars::HTTP_TIMEOUT)?)
let http_timeout = parse_u64_environment_variable(EnvVars::UV_HTTP_TIMEOUT)?
.or(parse_u64_environment_variable(EnvVars::UV_REQUEST_TIMEOUT)?)
.or(parse_u64_environment_variable(EnvVars::HTTP_TIMEOUT)?)
.map(Duration::from_secs);
Ok(Self {
@ -602,6 +609,15 @@ impl EnvironmentOptions {
python_install_registry: parse_boolish_environment_variable(
EnvVars::UV_PYTHON_INSTALL_REGISTRY,
)?,
concurrency: Concurrency {
downloads: parse_non_zero_usize_environment_variable(
EnvVars::UV_CONCURRENT_DOWNLOADS,
)?,
builds: parse_non_zero_usize_environment_variable(EnvVars::UV_CONCURRENT_BUILDS)?,
installs: parse_non_zero_usize_environment_variable(
EnvVars::UV_CONCURRENT_INSTALLS,
)?,
},
install_mirrors: PythonInstallMirrors {
python_install_mirror: parse_string_environment_variable(
EnvVars::UV_PYTHON_INSTALL_MIRROR,
@ -614,12 +630,10 @@ impl EnvironmentOptions {
)?,
},
log_context: parse_boolish_environment_variable(EnvVars::UV_LOG_CONTEXT)?,
upload_http_timeout: parse_integer_environment_variable(
EnvVars::UV_UPLOAD_HTTP_TIMEOUT,
)?
.map(Duration::from_secs)
.or(http_timeout)
.unwrap_or(Duration::from_secs(15 * 60)),
upload_http_timeout: parse_u64_environment_variable(EnvVars::UV_UPLOAD_HTTP_TIMEOUT)?
.map(Duration::from_secs)
.or(http_timeout)
.unwrap_or(Duration::from_secs(15 * 60)),
http_timeout: http_timeout.unwrap_or(Duration::from_secs(30)),
#[cfg(feature = "tracing-durations-export")]
tracing_durations_file: parse_path_environment_variable(
@ -702,8 +716,14 @@ fn parse_string_environment_variable(name: &'static str) -> Result<Option<String
}
}
/// Parse a integer environment variable.
fn parse_integer_environment_variable(name: &'static str) -> Result<Option<u64>, Error> {
fn parse_integer_environment_variable<T>(
name: &'static str,
err_msg: &'static str,
) -> Result<Option<T>, Error>
where
T: std::str::FromStr + Copy,
<T as std::str::FromStr>::Err: std::fmt::Display,
{
let value = match std::env::var(name) {
Ok(v) => v,
Err(e) => {
@ -712,7 +732,7 @@ fn parse_integer_environment_variable(name: &'static str) -> Result<Option<u64>,
std::env::VarError::NotUnicode(err) => Err(Error::InvalidEnvironmentVariable {
name: name.to_string(),
value: err.to_string_lossy().to_string(),
err: "expected an integer".to_string(),
err: err_msg.to_string(),
}),
};
}
@ -720,16 +740,29 @@ fn parse_integer_environment_variable(name: &'static str) -> Result<Option<u64>,
if value.is_empty() {
return Ok(None);
}
match value.parse::<u64>() {
match value.parse::<T>() {
Ok(v) => Ok(Some(v)),
Err(_) => Err(Error::InvalidEnvironmentVariable {
name: name.to_string(),
value,
err: "expected an integer".to_string(),
err: err_msg.to_string(),
}),
}
}
/// Parse a integer environment variable.
fn parse_u64_environment_variable(name: &'static str) -> Result<Option<u64>, Error> {
parse_integer_environment_variable(name, "expected an integer")
}
/// Parse a non-zero usize environment variable.
fn parse_non_zero_usize_environment_variable(
name: &'static str,
) -> Result<Option<NonZeroUsize>, Error> {
parse_integer_environment_variable(name, "expected a non-zero positive integer")
}
#[cfg(feature = "tracing-durations-export")]
/// Parse a path environment variable.
fn parse_path_environment_variable(name: &'static str) -> Option<PathBuf> {

View File

@ -117,15 +117,21 @@ impl GlobalSettings {
},
network_settings,
concurrency: Concurrency {
downloads: env(env::CONCURRENT_DOWNLOADS)
downloads: environment
.concurrency
.downloads
.combine(workspace.and_then(|workspace| workspace.globals.concurrent_downloads))
.map(NonZeroUsize::get)
.unwrap_or(Concurrency::DEFAULT_DOWNLOADS),
builds: env(env::CONCURRENT_BUILDS)
builds: environment
.concurrency
.builds
.combine(workspace.and_then(|workspace| workspace.globals.concurrent_builds))
.map(NonZeroUsize::get)
.unwrap_or_else(Concurrency::threads),
installs: env(env::CONCURRENT_INSTALLS)
installs: environment
.concurrency
.installs
.combine(workspace.and_then(|workspace| workspace.globals.concurrent_installs))
.map(NonZeroUsize::get)
.unwrap_or_else(Concurrency::threads),
@ -3747,16 +3753,6 @@ impl AuthLoginSettings {
// Environment variables that are not exposed as CLI arguments.
mod env {
use uv_static::EnvVars;
pub(super) const CONCURRENT_DOWNLOADS: (&str, &str) =
(EnvVars::UV_CONCURRENT_DOWNLOADS, "a non-zero integer");
pub(super) const CONCURRENT_BUILDS: (&str, &str) =
(EnvVars::UV_CONCURRENT_BUILDS, "a non-zero integer");
pub(super) const CONCURRENT_INSTALLS: (&str, &str) =
(EnvVars::UV_CONCURRENT_INSTALLS, "a non-zero integer");
pub(super) const UV_PYTHON_DOWNLOADS: (&str, &str) = (
EnvVars::UV_PYTHON_DOWNLOADS,
"one of 'auto', 'true', 'manual', 'never', or 'false'",

View File

@ -135,6 +135,15 @@ impl TestContext {
self
}
/// Set the "concurrent installs" for all commands in this context.
pub fn with_concurrent_installs(mut self, concurrent_installs: &str) -> Self {
self.extra_env.push((
EnvVars::UV_CONCURRENT_INSTALLS.into(),
concurrent_installs.into(),
));
self
}
/// Add extra standard filtering for messages like "Resolved 10 packages" which
/// can differ between platforms.
///

View File

@ -900,6 +900,24 @@ fn create_venv_with_invalid_http_timeout() {
"###);
}
#[test]
fn create_venv_with_invalid_concurrent_installs() {
let context = TestContext::new_with_versions(&["3.12"]).with_concurrent_installs("0");
// Create a virtual environment at `.venv`.
uv_snapshot!(context.filters(), context.venv()
.arg(context.venv.as_os_str())
.arg("--python")
.arg("3.12"), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: Failed to parse environment variable `UV_CONCURRENT_INSTALLS` with invalid value `0`: expected a non-zero positive integer
"###);
}
#[test]
fn create_venv_unknown_python_minor() {
let context = TestContext::new_with_versions(&["3.12"]).with_filtered_python_sources();