Skip to content

Collapse Docker pull progress in non-TTY output - #365

Open
sdairs wants to merge 2 commits into
issue-327-stop-all-enginesfrom
issue-353-docker-pull-progress
Open

Collapse Docker pull progress in non-TTY output#365
sdairs wants to merge 2 commits into
issue-327-stop-all-enginesfrom
issue-353-docker-pull-progress

Conversation

@sdairs

@sdairs sdairs commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Closes #353

Summary

  • collapse Docker image pull events into one image-level success or failure line for non-TTY, JSON, agent, and CI output
  • retain detailed layer IDs and byte progress for interactive terminals
  • preserve Docker diagnostics and exit status on pull failures

Tests

  • cargo fmt --all
  • cargo test -p clickhousectl local::docker::tests
  • cargo test -p clickhousectl --test local_docker_pull_progress_test
  • cargo test -p clickhousectl
  • cargo clippy -p clickhousectl --all-targets -- -D warnings

@sdairs
sdairs requested review from iskakaushik and rndD as code owners August 5, 2026 12:25
@sdairs sdairs changed the title Collapse non-TTY Docker pull progress (#353) Collapse Docker pull progress in non-TTY output Aug 5, 2026
@sdairs
sdairs force-pushed the issue-353-docker-pull-progress branch from f4da4b9 to ad6bd36 Compare August 5, 2026 13:42
@sdairs
sdairs requested a review from Copilot August 5, 2026 16:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves clickhousectl’s local Postgres Docker image pull reporting by collapsing noisy per-layer progress events into a single bounded summary line when output is non-interactive (non-TTY, JSON/agent/CI), while preserving detailed per-layer context for interactive terminals. This aligns local-command UX with the expectations in issue #353 and keeps Docker failure diagnostics intact.

Changes:

  • Add a PullReporter and progress-mode selection to switch between interactive (detailed) and collapsed (single-line) pull reporting.
  • Route local Postgres install/start image pulls through the new docker::pull_image(..., structured_output) behavior (removing ad-hoc eprintln! callers).
  • Add subprocess + unit tests validating collapsed non-TTY output and failure/diagnostic preservation.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
crates/clickhousectl/src/local/docker.rs Introduces interactive vs collapsed pull progress reporting and updates pull_image to emit bounded non-TTY output while keeping interactive detail.
crates/clickhousectl/src/local/mod.rs Updates local Postgres install path to use the new pull_image(..., json) behavior instead of printing progress in the caller.
crates/clickhousectl/src/local/postgres.rs Updates local Postgres start path to use the new pull_image(..., json) behavior when the image is missing.
crates/clickhousectl/tests/local_docker_pull_progress_test.rs Adds subprocess coverage using a fake Docker socket to assert collapsed non-TTY output and preserved diagnostics on pull failure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sdairs
sdairs force-pushed the issue-353-docker-pull-progress branch from 2f8c1e6 to 73fa2d4 Compare August 5, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Collapse Docker pull progress in non-TTY output

2 participants