feat(ampup): TTY/CI progress reporting for download manager#6
Merged
feat(ampup): TTY/CI progress reporting for download manager#6
Conversation
Introduce aggregate progress reporting so users see per-component status during parallel downloads, adapting output to the terminal environment. - Add `ProgressReporter` trait with `TtyProgress` (in-place line updates) and `CiProgress` (append-only) implementations - Auto-detect TTY via `console::Term::stderr().is_term()` in `create_reporter()` factory - Thread reporter through `download_all` via `Arc<dyn ProgressReporter>` with lifecycle callbacks - Add `download_error_artifact_name` helper to extract artifact name from any `DownloadError` variant - Add `NoopReporter` test helper and 16 unit tests covering state transitions and formatting Signed-off-by: Mitchell Spencer <mitchellhspencer@gmail.com>
…sBar Decouple progress bar lifecycle from download logic by accepting a ProgressBar parameter instead of a boolean toggle, enabling callers to control styling and completion behavior. - Replace `show_progress: bool` with `progress_bar: ProgressBar` across all download methods in `github.rs` - Move progress bar creation and finish logic into `updater.rs` for self-update downloads - Pass `ProgressBar::hidden()` in `download_manager.rs` to suppress per-file bars (aggregate reporting via `ProgressReporter`) - Remove progress bar creation and styling code from `github.rs` Signed-off-by: Mitchell Spencer <mitchellhspencer@gmail.com>
…ogress reporter - Replace all Mutex::lock().expect() calls with unwrap_or_else(|e| e.into_inner()) so a poisoned lock is recovered from rather than crashing the download - Replace raw println!/eprintln! in CiProgress with crate::ui::success!/warn! macros to comply with the CLI output pattern - Replace .expect() in updater.rs with .context()? for proper error propagation - Improve test .expect() messages from "lock" to descriptive strings
The original issue only required per-component status labels (waiting/downloading/downloaded), not byte-level bars. Remove indicatif to simplify the download path. - Remove `indicatif` dependency from `Cargo.toml` - Drop `progress_bar: ProgressBar` parameter from all download methods in `github.rs` - Rename `download_with_progress` to `download_response` — no longer tracks bytes - Remove `ProgressBar` spinner and byte-tracking from `updater.rs` - Remove `ProgressBar::hidden()` plumbing from `download_manager.rs` Signed-off-by: Mitchell Spencer <mitchellhspencer@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements progress reporting for the
ampup installcommand as part of issue edgeandnode/amp#1762.Before this PR, installs ran silently with no feedback. Now:
waiting...→→ downloading...→✓ downloaded✓ [1/2] Downloaded ampdThe reporter is auto-detected at runtime via
Term::stderr().is_term().Changes
progress.rs(new) — defines theProgressReportertrait and its two implementations:TtyProgressfor interactive terminals andCiProgressfor CI/piped output. Acreate_reporter()factory picks the right one automatically.github.rs— refactored download methods fromshow_progress: booltoprogress_bar: ProgressBar. Callers now own the progress bar lifecycle;github.rsonly updates position during streaming.download_manager.rs— wires the reporter through the concurrent download pipeline. Tasks reportstarted/completed/failedat the right points, including during fail-fast shutdown.install.rs— creates the reporter and passes it intodownload_all.updater.rs— self-update now creates its own styledProgressBarfor the single binary download it performs.