Skip to content

feat(pb): use an indeterminate spinner for the push checking phase - #680

Open
aftersnow wants to merge 1 commit into
mainfrom
feat/push-checking-spinner
Open

aftersnow wants to merge 1 commit into
mainfrom
feat/push-checking-spinner

Conversation

@aftersnow

Copy link
Copy Markdown
Contributor

Problem

Follow-up to #474. The Copilot review on #474 (#474 (comment)) found that the checking phase still renders a zero transfer rate. #474 was merged before the fix landed on its branch, so this PR carries the fix.

The checking phase created a transfer bar with a nil reader. That bar has byte counters and an AverageSpeed decorator, but nothing advances them. A slow dst.Exists call therefore showed 0 B / <size> and 0 B/s under "Checking ...".

Change

  • internal/pb: add ProgressBar.Spinner(prompt, name, size). It creates an indeterminate bar (total = 0, so mpb never triggers completion on its own) with a spinner filler, the prompt, the total size and the elapsed time. It has no byte counter and no speed decorator. Reset or Add replace it with a transfer bar once bytes flow. Complete gives a spinner a total so mpb can mark it done.
  • pkg/backend/push.go: the checking phase calls pb.Spinner(...) instead of pb.Add(..., nil).
  • Tests: TestSpinner_* cover creation, replacement by Reset, completion, the disabled path, and the rendered output (no counter, no rate). TestAdd_RendersTransferRate is the sanity check that a transfer bar with no bytes flowing does render 0.00 b / N b | 0.00 b/s, so the spinner assertion is not vacuous.

Verification

go test -race -count=1 ./internal/pb/ ./pkg/backend/
ok  	github.com/modelpack/modctl/internal/pb	1.878s
ok  	github.com/modelpack/modctl/pkg/backend	1.611s
golangci-lint run   # 0 issues

Manual end-to-end check against a local registry:3.1.2 behind a proxy that delays HEAD /v2/*/blobs/* by 2 s. Rendered output in a 220-column pty:

Checking blob => sha256:<digest> ⠋ 1.2 MB | 0s
Checking blob => sha256:<digest> ⠇ 1.2 MB | 1s
Pushing blob  => sha256:<digest> | 1.2 MB | done(0.1s)

No Checking frame contains b/s.

The checking phase created a transfer bar with a nil reader. That bar has
byte counters and an AverageSpeed decorator, but nothing advances them, so
a slow dst.Exists call rendered "0 B / <size>" and "0 B/s" under
"Checking ...", which is the zero transfer rate this PR removes.

Add ProgressBar.Spinner: an indeterminate bar with a spinner filler, the
prompt, the total size, and the elapsed time, and no byte or speed
decorators. push uses it for the checking phase; Reset then replaces it
with the transfer bar once the upload starts. Complete gives a spinner a
total so mpb can mark it done.

Tests cover creation, replacement by Reset, completion, the disabled
path, and the rendered output (no counter, no rate), with a sanity test
that a transfer bar does render the rate.

Signed-off-by: Zhao Chen <zhaochen.zju@gmail.com>
(cherry picked from commit 4382099)

This branch has not been deployed

No deployments
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.

1 participant