-
Notifications
You must be signed in to change notification settings - Fork 499
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
Output: use logging, remove verbose output by default #741
Conversation
f5e5dfe
to
4791476
Compare
I made the linter happy, so this can be merged |
@muayyad-alsadi can I do anything to help getting this merged? |
This would be extremely useful. |
+1 |
@maurerle Maybe remove the word "garbage" and rename it to "verbose"? 😅 I really hope this gets merged soon. |
I too hope this can be merged soon. It's a pretty minor code change for a massive QoL improvement. |
Just starting to evaluate migrating to podman from docker. It's a little alarming that there is not yet a way to lower the noticeably much more verbose output, but I'll keep evaluating it. |
Hi @ParetoOptimalDev, Other than that you can try my fork of podman-compose which has merged a lot of the missing features which are available in open pull requests here: |
any news? 👀 |
I think this project is abandoned.
…On Wed, 24 Jan 2024, 15:03 Thiago Fortunato, ***@***.***> wrote:
any news? 👀
—
Reply to this email directly, view it on GitHub
<#741 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABOJKNPLUAJZTWMTJTBNJV3YQEPFPAVCNFSM6AAAAAA3HL2DIOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBYGMYTGOJYGQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
@@ -394,7 +392,7 @@ def assert_volume(compose, mount_dict): | |||
proj_name = compose.project_name | |||
vol_name = vol["name"] | |||
is_ext = vol.get("external", None) | |||
log(f"podman volume inspect {vol_name} || podman volume create {vol_name}") | |||
log.debug("podman volume inspect %s || podman volume create %s", vol_name, vol_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to keep f-string str.format formatting here? I understand that debug function would still need to parse it internally. It feels inconsistent to see {}
in some places and then %
in others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review!
There is no better approach to this as I know.
It is best practice to lazy eval in logging (which is currently only supported with % placeholders) but it is efficient to use f-strings everywhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you!
The PR will need a rebase to latest main branch.
cc @maurerle or anyone else who's interested. This PR could easily land if someone rebased it. |
Fixes containers#489. Introduces a --verbose flag if you want to see all the noise that was previously printed by default. Signed-off-by: James O'Beirne <[email protected]> Signed-off-by: Florian Maurer <[email protected]>
* fix black and pylint issues Signed-off-by: Florian Maurer <[email protected]>
Thanks @p12tic ! hopefully this will get merged finally :) |
We may need to re-add part of logging that prepended task name to log output. However this is useful only to podman-compose developers. There are many more users who will find their output much cleaner now. So it makes sense to postpone further improvements and merge what we have. |
Fixes #489.
Introduces a --verbose flag if you want to see all the noise that was previously printed by default.