-
-
Notifications
You must be signed in to change notification settings - Fork 232
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Include notes when logging exceptions #684
Conversation
This adds support for the notes feature from https://peps.python.org/pep-0678/, which was introduced in Python 3.11.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
src/structlog/tracebacks.py
Outdated
""" | ||
|
||
exc_type: str | ||
exc_value: str | ||
exc_notes: str | None = None |
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.
hey sorry for the long delay – is there a reason why you chose to make it an optional str?
You mention it in the description but without reasoning and I think it would be more ergonomic to make it a tuple[str, ...]
?
(tuple > list because it would keep Stack hashable)
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.
I forget; I think I was concerned about how it would show up in a particular system we have, but for no good reason. A tuple works better. Done! Sorry I lost track of this for a while.
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.
thanks! but ugh I'm so dumb, there already is a list in the class so it makes no sense to make this inconsistently a list. I'll fix it myself, this is on me.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Summary
This adds support for the notes feature from https://peps.python.org/pep-0678/, which was introduced in Python 3.11.
These notes are not included in
str
, but they are included intraceback.print_exception
, etc.; we should include them when logging, too.Note: an alternative option for formatting would be to include these in the
exc_value
string.Pull Request Check List
main
branch – use a separate branch!api.py
.docs/api.rst
by hand.versionadded
,versionchanged
, ordeprecated
directives..rst
and.md
files is written using semantic newlines.