-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Show messages for slow tasks in non-interactive mode #11165
Conversation
When stderr is not a tty, we currently don't show any messages for build or large downloads, since indicatif is hidden. We can improve this by showing a message for: * Starting and finishing a large download (>1MB) * Starting and finishing a build Downloads are limited to 1MB or unknown size to keep the logs concise and not scroll the entire terminal away for a download that finishes almost immediately. Part of #11121 ** Test Plan** ``` $ uv venv && FORCE_COLOR=1 cargo run -q pip install numpy --no-binary :all: --no-cache 2>&1 | tee a.txt Using CPython 3.13.0 Creating virtual environment at: .venv Activate with: source .venv/bin/activate Resolved 1 package in 221ms Building numpy==2.2.2 Built numpy==2.2.2 Prepared 1 package in 2m 34s Installed 1 package in 6ms + numpy==2.2.2 ``` ``` $ uv venv && FORCE_COLOR=1 cargo run -q pip install torch --no-cache 2>&1 | tee b.txt Using CPython 3.13.0 Creating virtual environment at: .venv Activate with: source .venv/bin/activate Resolved 24 packages in 648ms Downloading setuptools (1.2MiB) Downloading nvidia-cuda-cupti-cu12 (13.2MiB) Downloading torch (731.1MiB) Downloading nvidia-nvjitlink-cu12 (20.1MiB) Downloading nvidia-cufft-cu12 (201.7MiB) Downloading nvidia-cuda-nvrtc-cu12 (23.5MiB) Downloading nvidia-curand-cu12 (53.7MiB) Downloading nvidia-nccl-cu12 (179.9MiB) Downloading nvidia-cudnn-cu12 (634.0MiB) Downloading nvidia-cublas-cu12 (346.6MiB) Downloading sympy (5.9MiB) Downloading nvidia-cusparse-cu12 (197.8MiB) Downloading nvidia-cusparselt-cu12 (143.1MiB) Downloading networkx (1.6MiB) Downloading nvidia-cusolver-cu12 (122.0MiB) Downloading triton (241.4MiB) Downloaded setuptools Downloaded networkx Downloaded sympy Downloaded nvidia-cuda-cupti-cu12 Downloaded nvidia-nvjitlink-cu12 Downloaded nvidia-cuda-nvrtc-cu12 Downloaded nvidia-curand-cu12 [...] ```
@@ -120,7 +120,7 @@ impl GitSource { | |||
// Report the checkout operation to the reporter. | |||
if let Some(task) = task { | |||
if let Some(reporter) = self.reporter.as_ref() { | |||
reporter.on_checkout_complete(remote.url(), short_id.as_str(), task); | |||
reporter.on_checkout_complete(remote.url(), actual_rev.as_str(), task); |
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.
This change avoids the start messages showing the long hash, while the finish message shows the short hash.
)); | ||
); | ||
if multi_progress.is_hidden() && !*HAS_UV_TEST_NO_CLI_PROGRESS { | ||
let _ = writeln!(self.printer.stderr(), "{message}"); |
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 could raise the errors here if we need.
return; | ||
}; | ||
|
||
let progress = state.lock().unwrap().bars.remove(&id).unwrap(); | ||
|
||
let size = state.lock().unwrap().download_size[&id]; |
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.
Should we not grab both of these with a single lock acquisition?
@@ -154,15 +175,36 @@ impl ProgressReporter { | |||
.unwrap() | |||
.progress_chars("--"), | |||
); | |||
// If the download is larger than 1MB, show a message to indicate that this may take | |||
// a while keeping the log concise. | |||
if multi_progress.is_hidden() && !*HAS_UV_TEST_NO_CLI_PROGRESS && size > 1024 * 1024 { |
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.
Hmm, it's a bit strange that we hide these but always show them if we don't have a size? I'd expect them to be always visible.
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 don't quite follow your comment. You expect them not to be gated by size? (I sort of agree, since it's not in a tty anyway it seems fine to write them all) Or you expect them to not be gated by the multiprogress status?
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 expect them not to be gated by size, i.e., always shown. (In the else
branch, where we don't have a size, we always show them, so that discrepancy seemed odd to me.)
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.
Let's track in #11272
This is very helpful and adding tick messages would be even better! For large downloads like pytorch cuda, this would still seem like uv is stuck on downloading this step since it takes so long. |
I will still insist a JSON output is the solution for this. I don't think we can deliver nice human readable tick messages — that's why we have progress bars in the interactive output. |
When stderr is not a tty, we currently don't show any messages for build or large downloads, since indicatif is hidden. We can improve this by showing a message for:
Downloads are limited to 1MB or unknown size to keep the logs concise and not scroll the entire terminal away for a download that finishes almost immediately.
These messages are not captured in the tests since their order is non-deterministic (downloads and builds race to finish).
There are no "tick" messages for large downloads yet, we could e.g. show an update on runnning downloads every n seconds.
Part of #11121
Test Plan