Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34k
gh-68443: Replace debug level-related logic in http client with logging#8633
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
CuriousLearner commentedAug 7, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
CuriousLearner commentedMay 9, 2019
Hey@bitdancer ,@vsajip Can you please have a look at this? I just noticed this PR wasn't reviewed from almost a year. Please let me know, what is needed here. Thanks! |
willingc commentedMay 9, 2019
@CuriousLearner I would go ahead and rebase to resolve conflicts. |
CuriousLearner commentedMay 11, 2019
@willingc Done! |
Lib/http/client.py Outdated
| raise LineTooLong("status line") | ||
| if self.debuglevel > 0: | ||
| print("reply:",repr(line)) | ||
| _log.info("reply: {}".format(repr(line))) |
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.
logging first argument support old format string, so_log.info("reply: %s", repr(line)) would work. There is mixed calls of{}".format(foo with%s", foo, wouldn't be better to stick with one?
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.
wouldn't be better to stick with one?
I agree -% should work fine and a lot of logging uses %-formatting rather than.format() due to its age.
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.
There is another reason: tools like Sentry use message pattern as a key for logs grouping.
Adding pre-formatted messages to Sentry is very ineffective.
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.
Ah, sorry!
I think I came back to this later and followed the same style as for other Python projects I was involved with.
Fixing this!
Lib/test/test_httplib.py Outdated
| import io | ||
| import itertools | ||
| import os | ||
| import sys |
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.
This isn't being used anywhere!?
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.
Yeah, good catch. I think it remained here while I was working on this.
Lib/test/test_logging.py Outdated
| logger = logging.getLogger("http") | ||
| root_logger = self.root_logger | ||
| root_logger.removeHandler(self.root_logger.handlers[0]) | ||
| logger = logging.getLogger("httplogger") |
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.
Why this name change?
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.
There is no specific reason for it. I'll just revert back.
This PR is stale because it has been open for 30 days with no activity. |
This PR is stale because it has been open for 30 days with no activity. |
python-cla-botbot commentedApr 18, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
All commit authors signed the Contributor License Agreement. |
This PR is stale because it has been open for 30 days with no activity. |
This PR is stale because it has been open for 30 days with no activity. |
9933ad5 tob6997e1CompareReplace debuglevel-based print statements with Python's standardlogging module for better integration with logging frameworks andlog aggregation tools like Sentry. Uses % formatting consistentlyfor better log pattern matching in aggregation tools.
c240289 toafeb73cCompareThis PR is stale because it has been open for 30 days with no activity. |
Uh oh!
There was an error while loading.Please reload this page.
Hey@bitdancer , here's the PR for issue:https://bugs.python.org/issue24255
https://bugs.python.org/issue24255