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

[BUG] For up command, "pulling" and "Pulled" events should respect quietPull flag #10686

Closed
Songyu-Wang opened this issue Jun 9, 2023 · 7 comments
Labels

Comments

@Songyu-Wang
Copy link

Description

The function pullServiceImage in https://github.com/docker/compose/blob/v2/pkg/compose/pull.go has quietPull bool as an argument.

However, the "pulling" and "Pulled" event logger are not wrapped under the argument.
It should follow the same logic as

if !quietPull {
			toPullProgressEvent(service.Name, jm, w)
		}

Steps To Reproduce

N/A

Compose Version

v2 head

Docker Environment

N/A

Anything else?

image

@Songyu-Wang Songyu-Wang changed the title [BUG] For up command, "pulling" and "Pulled" should respect quietPull flag [BUG] For up command, "pulling" and "Pulled" events should respect quietPull flag Jun 9, 2023
@ndeloof
Copy link
Contributor

ndeloof commented Jun 10, 2023

quietPull allows to skip individual pull progress messages to be displayed, but still we want the user to be aware about the current state of a service, which will be Pulling before it can become Created then Started.

@Songyu-Wang
Copy link
Author

Songyu-Wang commented Jun 10, 2023

@ndeloof
Thanks for replying.
I think [+] Running 1/15 would be more than enough to indicate current state of a service. Pulling and pulled events are just redundant information.

I am gonna assume most people use quiet pull because they are using docker compose with some kinda SaaS CI and the interactive logs keep getting duplicated every seconds(10th of a second?), For example, https://github.com/docker/compose/actions/runs/5220651859/jobs/9423987875 is not the exact commands, but the symptom is the same.
image

Only removing part of the messages but keeping others wont resolve the above pain point.

I would really hope you can re-consider it

@ndeloof
Copy link
Contributor

ndeloof commented Jun 11, 2023

Only removing part of the messages but keeping others wont resolve the above pain point.

The only remaining parts are Pulling then Pulled, just 2 lines when ran in "non-TTY" mode (which may generates verbose logs), which are useful to understand the pull completed successfully.

$ docker compose --ansi=never up --quiet-pull
 test Pulling 
 test Pulled 
 Container truc-test-1  Creating
 Container truc-test-1  Created
Attaching to truc-test-1

I'm closing this as "not planned"

@ndeloof ndeloof closed this as not planned Won't fix, can't repro, duplicate, stale Jun 11, 2023
@Songyu-Wang
Copy link
Author

image

@ndeloof
Copy link
Contributor

ndeloof commented Jun 11, 2023

This happens because you use ANSI-based progress without an actual terminal. A new ANSI sequence will be emitted every 100ms, and obviously a pull operation can take much more. Are you forcing --ansi flag here ?

@Songyu-Wang
Copy link
Author

Songyu-Wang commented Jun 11, 2023

We are not because the ci tool we are using handles ansi on the web UI. The issue is the size of exported logs from the said ui tool( which is just a plain text file)

@ndeloof
Copy link
Contributor

ndeloof commented Jun 12, 2023

I proposed we introduce a flag to offer better control on the progress UI, so you can switch to plain text mode and avoid huge logs, but still can benefits from ANSI-colored output for container logs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants