-
Notifications
You must be signed in to change notification settings - Fork 56
Linting take 1 #19
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
Linting take 1 #19
Conversation
verifier.verify(signature) | ||
except Exception: | ||
message = 'Unable to verify signature' | ||
_LOGGER.exception(message) |
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.
Would it make sense to include the message (perhaps trimmed to a certain length) in the log to help with debugging?
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.
When the "exception" logging level is used in the scope of an except
block, it automatically logs the full stacktrace of the exception that caused that block to be entered.
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.
But that stack trace wouldn't contain the message its signature that we failed to verify, right?
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.
That's a very good point. Opened #26 to track this, because I think that would be very useful.
Apparently it also needed to be rebased after master was merged...ok then. |
Code formatting/style/smell fixes identified by flake8/pylint.
BLOCKED BY #16
BLOCKED BY #18
NOTE
To ease parallelization of efforts, this PR is based on the
DELETE-m-1
fork. Prior to looking over this PR, #16 and #18 should be merged tomaster
and the base for this PR be changed tomaster
.