copy: add --progress-interval for CI-compatible progress output#2870
copy: add --progress-interval for CI-compatible progress output#2870IanEff wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Thanks!
Just a very quick look now: I think this is just the wrong place, and the way this needs to completely turn off ReportWriter is an indication of that. If we can make the c/image output better, that improves all callers, including notably Podman, and we can design a reasonable output there; maintaining code that writes progress reports in one repo and code that discards those progress reports in another repo seems rather unnecessary.
|
|
||
| if opts.progressInterval > 0 { | ||
| copyOpts.ProgressInterval = opts.progressInterval | ||
| copyOpts.ReportWriter = nil // suppress TTY bar when logging to stderr |
There was a problem hiding this comment.
I don’t see where stderr figures in this logic at all. Is that just due to resolveProgressInterval deciding based on that?
ReportWriter contains quite a bit of other information, not just blob progress, so completely turning it off is not great.
| blobs := map[string]*blobState{} | ||
|
|
||
| for props := range ch { | ||
| dig := props.Artifact.Digest.Encoded() |
There was a problem hiding this comment.
Encoded does not include the algorithm , that’s inaccurate as a map key.
|
|
||
| case types.ProgressEventRead: | ||
| b := blobs[dig] | ||
| bytesPerSec := float64(props.OffsetUpdate) / interval.Seconds() |
There was a problem hiding this comment.
??? interval.Seconds is a constant throughout the life of the function.
IanEff
force-pushed
the
ci-progress-output
branch
from
9223af2 to
ab233b1
Compare
|
@IanEff looks like this needs a rebase. |
IanEff
force-pushed
the
ci-progress-output
branch
from
ab233b1 to
399e79d
Compare
In a pipe, skopeo copy was completely silent until success or failure. Wire the progress channel exposed by containers/image to emit timestamped progress updates to stderr suitable for CI logging. Auto-detects non-TTY environments. Suppressed by --quiet. Closes podman-container-tools#658 Ian Furst <ian.furst@gmail.com> Signed-off-by: Ian Furst <ian.furst@gmail.com>
IanEff
force-pushed
the
ci-progress-output
branch
from
399e79d to
eb926b4
Compare
|
@mtrmac I'm inclined to agree. I'll abandon this and go sniffing upstream in c/image, make the changes there. Thank you for your feedback on my commit! @TomSweeneyRedHat Thanks! |
3 participants
In a pipe, skopeo copy was silent until success or failure.
Add
--progress-intervalto thecopycommand. When stderr is not a TTY, periodic timestamps are emitted to stderr with current blob's offset, total size, and transfer rate. Defaults to 30s in non-TTY environments, & is disabled when a TTY is detected or when --quiet is passed.Closes #658