-
Notifications
You must be signed in to change notification settings - Fork 0
Exception context #60
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
Conversation
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Co-authored-by: Marvin Poul <ponder@creshal.de> Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #60 +/- ##
==========================================
+ Coverage 95.12% 95.34% +0.22%
==========================================
Files 14 15 +1
Lines 574 602 +28
==========================================
+ Hits 546 574 +28
Misses 28 28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
not BaseException; for consistency with type hints and between tools Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Logic is unchanged, but meaning is clearer Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
on garbage input Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
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.
Left a suggestion for docs, but otherwise looks good to me.
| ... history.append(message) | ||
|
|
||
| Callback on all exceptions when no types are specified: |
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.
Maybe a simple example that shows the use as a plain decorator would be nice, too.
>>> try:
... with on_error(print, RuntimeError, 42):
... raise RuntimeError()
... except RuntimeError:
... pass
42At least I expect to get the most mileage out of this incarnation.
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, absolutely. I had misunderstood how much power @contextlib.contextmanager was giving us -- I thought we additionally had to use it inside a regular exit stack.
I agree, many applications will need only a single callback, for which this is a much more condensed syntax 👍
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Closes #59
I want to play around using it as a decorator, but I'm not yet convinced that the signature will be clean enough to be useful.
EDIT: I don't have time to play around with a decorator format. I'm not closed to it, I'd just rather leave it for later and already get access to what's here.