Skip to content

Set default logging level to WARNING #1178

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jan 1, 2024
Merged

Set default logging level to WARNING #1178

merged 5 commits into from
Jan 1, 2024

Conversation

dukecat0
Copy link
Member

  • I have added an entry to docs/changelog.md

Summary of changes

Set the default logging level to WARNING, so debug log messages won't be shown without passing additional flags such as --verbose.

Closes #1175
Closes #1177

Test plan

Tested by running

nox -s tests-3.12

chrysle
chrysle reviewed Dec 30, 2023
View reviewed changes
@dukecat0
Copy link
Member Author

dukecat0 commented Jan 1, 2024

Probably we need to update this line to ERROR and CRITICAL logging levels.?

pipx/src/pipx/main.py

Line 720 in 81c2fa5

" WARNING, ERROR, and CRITICAL logging levels."

@chrysle
Copy link
Contributor

chrysle commented Jan 1, 2024

Yes, looks like it.

@mayeut
Copy link
Member

mayeut commented Jan 1, 2024

The "-q" added in #1159 shall also be removed:

pipx/tests/test_list.py

Line 83 in 81c2fa5

assert not run_pipx_cli(["list", "-q", "--json"])

@dukecat0
Copy link
Member Author

dukecat0 commented Jan 1, 2024

We can change its behavior instead of removing it. I remember the original usage of this flag was hiding all console outputs, not just the debug log messages.

@dukecat0 dukecat0 merged commit 6d47060 into pypa:main Jan 1, 2024
@dukecat0 dukecat0 deleted the fix-logging-lvl branch January 1, 2024 13:23
@mayeut
Copy link
Member

mayeut commented Jan 1, 2024

We can change its behavior instead of removing it. I remember the original usage of this flag was hiding all console outputs, not just the debug log messages.

It might be so but:
IMHO, it should have raised a red flag in review (but can be easily missed).
Removing the flag in that test allows to see that the default log level is the correct one (for now, it seems it's the only test for that - although probably accidental).
Given this PR reverts the default log level to WARNING, I think we should also revert this change that was only introduced because of the default log level change.

If there are more changes to be made as you suggest, then it would be for another PR but for now, I think changing back this test is the best option.

@dukecat0 dukecat0 mentioned this pull request Jan 1, 2024
Merged
@dukecat0
Copy link
Member Author

dukecat0 commented Jan 1, 2024

I've opened #1181 to revert it now.

@ealap ealap mentioned this pull request Jan 2, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrysle chrysle chrysle approved these changes

@gaborbernat gaborbernat Awaiting requested review from gaborbernat

@uranusjr uranusjr Awaiting requested review from uranusjr

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Running pipx without any arguments errors on 1.4.0 Getting debug-like log messages in console after upgrade to v1.4.0
3 participants
@dukecat0 @chrysle @mayeut