-
Notifications
You must be signed in to change notification settings - Fork 118
added logging to HTTP errors w/ url, status and response content #77
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
base: master
Are you sure you want to change the base?
Conversation
pass | ||
|
||
logger = logging.getLogger(__name__) | ||
logger.addHandler(NullHandler()) |
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.
A library should not configure logging handlers. I would revert this.
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 is a common trick because the logging
library is stupid and shoves messages to stderr if there are no handlers present.
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.
iow if you don't do this trick, then end users are forced to configure logging if they don't want spurious output.
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.
If zero handlers are present, Python will yell every time the log line is hit with "No handlers could be found for logger "slumber"".
I've seen the NullHandler pattern in quite a few libs. In fact, I borrowed the above code directly out of requests.
I'm fine using whatever log level you deem appropriate. Obviously an easy change. :) |
@merwok I agree with the error level. There is a "slumber" logger, so it's configurable. |
OK for the null handler. @pior: are you saying you agree with the proposed error level? |
This change adds a "slumber" logger and uses it to emit ERROR level messages w/ the URL, HTTP Status and response content in the event of an HTTP error. This should make it easier to debug server errors on the client side.