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
[core]: Adds support for on_error to abort further event handlers on exceptions #220
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #220 +/- ##
========================================
Coverage ? 77.1%
========================================
Files ? 80
Lines ? 6700
Branches ? 0
========================================
Hits ? 5166
Misses ? 1534
Partials ? 0
Continue to review full report at Codecov.
|
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 need to have this flag also for the specific handler, is this possible?
elif event.on_error == "ignore": | ||
continue | ||
else: | ||
continue |
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.
Since strings are hard to handle (prone to typos), I think it would be better to check if on_error
is in the allowed options and raise otherwise. What about using constants for those strings?
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 am unsure about using constants because everybody needs to import them then.
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 still dislike to use constants because they have to be imported and would make API changes in the future harder.
You also have the same problem with the (in circuits currently widely used) setting of attributes to classes (e.g. handler.event = False
), which wouldn't raise any error.
@prologic Why do you close this and remove the branch? |
Ahh I was doing some Github cleanup! Sorry, do you want to take over the branch/PR? |
I can't promise to work on this currently, but I still would need it. I restore the branch to not remove the current working state. |
The following code demonstrates this behavior. As foo.on_error is 'throw_last' the on_error_1 handler is executed as well as on_error_2 which is the exception throwed back to the bar handler. from circuits import Event, Component, handler, Debugger import pytest class foo(Event): on_error = 'throw_last' class bar(Event): pass class Foo(Component): @handler('foo', priority=2) def _on_error_1(self): raise ValueError('test') @handler('foo', priority=1) def _on_error_2(self): raise ValueError('test2') def bar(self): with pytest.raises(ValueError) as exc: x = yield self.call(foo()) assert exc.value.args[0] == 'test2' print('DONE') x = Foo() x += Debugger() x.fire(bar()) x.run()
2ccd2f8
to
78e2cee
Compare
Closes #128