gh-68443: Replace debug level-related logic in http client with logging#8633
Uh oh!
There was an error while loading. Please reload this page.
Conversation
CuriousLearner commented May 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 commented May 9, 2019
@CuriousLearner I would go ahead and rebase to resolve conflicts. |
CuriousLearner commented May 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.
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.
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.
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.
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.
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.
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. |
picnixz changed the title 
github-actionsbot removed the stale This PR is stale because it has been open for 30 days with no activity. |
All commit authors signed the Contributor License Agreement. |
github-actionsbot removed the stale This PR is stale because it has been open for 30 days with no activity. |
github-actionsbot removed the stale This PR is stale because it has been open for 30 days with no activity. |
CuriousLearnerforce-pushed the fix-issue24255 branch from 9933ad5 to b6997e1Compare
Replace debuglevel-based print statements with Python's standard logging module for better integration with logging frameworks and log aggregation tools like Sentry. Uses % formatting consistently for better log pattern matching in aggregation tools.
CuriousLearnerforce-pushed the fix-issue24255 branch from c240289 to afeb73cCompareThis PR is stale because it has been open for 30 days with no activity. |
8 participants
Hey @bitdancer , here's the PR for issue: https://bugs.python.org/issue24255
https://bugs.python.org/issue24255