Skip to content
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

make progress title configurable #10507

Merged
merged 1 commit into from May 2, 2023
Merged

Conversation

glours
Copy link
Contributor

@glours glours commented Apr 28, 2023

What I did
Add a progress section title to tty writer so we're able to show a more accurate state than Running in the console output.
Screenshot 2023-04-28 at 12 15 39

This changes will also improve dry run output.

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

@glours glours self-assigned this Apr 28, 2023
@glours glours requested review from a team, nicksieger, ndeloof, StefanScherer, ulyssessouza, milas and laurazard and removed request for a team April 28, 2023 10:18
@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Patch coverage: 60.97% and project coverage change: -0.01 ⚠️

Comparison is base (4f2c933) 60.22% compared to head (7e65835) 60.21%.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10507      +/-   ##
==========================================
- Coverage   60.22%   60.21%   -0.01%     
==========================================
  Files         107      107              
  Lines        9201     9207       +6     
==========================================
+ Hits         5541     5544       +3     
- Misses       3094     3097       +3     
  Partials      566      566              
Impacted Files Coverage Δ
pkg/compose/kill.go 57.50% <0.00%> (ø)
pkg/compose/push.go 0.00% <0.00%> (ø)
pkg/progress/tty.go 39.72% <0.00%> (ø)
pkg/progress/writer.go 66.19% <45.00%> (+1.58%) ⬆️
pkg/compose/build.go 75.07% <100.00%> (ø)
pkg/compose/cp.go 60.60% <100.00%> (ø)
pkg/compose/create.go 61.41% <100.00%> (ø)
pkg/compose/pause.go 43.47% <100.00%> (ø)
pkg/compose/pull.go 76.72% <100.00%> (ø)
pkg/compose/remove.go 69.33% <100.00%> (ø)
... and 2 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ndeloof
Copy link
Contributor

ndeloof commented Apr 28, 2023

LGTM for a short terms fix

This is yet another occurence to demonstrate the progress mechanism is not adequate for our needs.
For example, on pull we don't distinguish the "download", "unpack" and "verify" phases, and then report 3 times a progression from 0 to 100% (hopefully, the 2 last steps are fast so nobody noticed :P).

Longer terms I think we should have components send structured events, not just strings, and define a real TUI to manage and format them. https://github.com/anchore/stereoscope has a nice UI, we should look into implementation for inspiration. Typically it fully parses the engine JSONMessages to convert them into structured events: https://github.com/anchore/stereoscope/tree/main/pkg/image/docker
(to be debated why moby doesn't send such events as an "API" and prefers to manage TUI from engine vs client, but ¯\(ツ)/¯)

Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com>
@milas milas merged commit 03f4c0e into docker:v2 May 2, 2023
23 of 24 checks passed
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.

None yet

3 participants