-
Notifications
You must be signed in to change notification settings - Fork 648
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
Control pip timeout duration via environment variable #1694
Conversation
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.
Thanks for contributing! I have a couple minor questions.
match value.parse::<u64>() { | ||
Ok(parsed) => parsed, | ||
Err(_) => { | ||
eprintln!("Warning: UV_REQUEST_TIMEOUT is an invalid value. Using default value."); |
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.
We have a warn_user_once!
macro that we should use instead here.
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.
Would it make sense to use .map_err
instead of a match
here?
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.
I'd also probably be a little clearer here, maybe:
Ignoring invalid value for UV_REQUEST_TIMEOUT. Expected integer number of seconds, got "{value}".
} | ||
Err(_) => default_timeout, | ||
}; | ||
debug!("Pip request timeout is {}.", timeout); |
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.
We should probably not say "Pip" this is our general registry client
debug!("Pip request timeout is {}.", timeout); | |
debug!("Using registry request timeout of {}s", timeout); |
@zanieb |
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.
Thanks!
…OUT` (#1780) Follow-up to #1694 matching Cargo's environment variable names https://doc.rust-lang.org/nightly/cargo/reference/config.html#httptimeout
Summary
Add the environment variable
UV_REQUEST_TIMEOUT
to allow control over pip timeouts.Closes #1549
Test Plan
I built uv in the repository top Dockerfile, set the timeout to 3 seconds, and ran
uv pip install torch
.I measured the execution time with the time command and confirmed that the process finished at a value close to the timeout we set.