Add parameter for on_error to BusABC.send_periodic#1283
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@## develop #1283 +/- ## ======================================== Coverage 66.02% 66.02% ======================================== Files 86 86 Lines 8915 8915 ======================================== Hits 5886 5886 Misses 3029 3029 |
If there is an error while sending messages in a (background) periodic task, then the `on_error` callback is called, if set. If the callback is configured and returns `True` then the task continues, otherwise it is aborted. While it is possible to update a task with a callback after the task has been created, this procedure is prone to a race condition where the callback might not be configured in time for the first send event. Thus, if the first send fails and the callback has not yet been configured, the task will abort. This commit solves the race condition issue by adding an argument to `BusABC.send_periodic` to specify the callback. By including the callback in the constructor it will be deterministically active for all sends in the task. This fixes issue hardbyte#1282. This commit also adds myself to the CONTRIBUTORS list.
andebjorforce-pushed the periodic_callback_arg branch from 3b8bb3c to a0e1f92Compare
| task = cast( | ||
| _SelfRemovingCyclicTask, | ||
| self._send_periodic_internal(msgs, period, duration), | ||
| self._send_periodic_internal(msgs, period, duration, on_error), |
Collaborator
There was a problem hiding this comment.
This will fail for subclasses that reimplement _send_periodic_internal()
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If there is an error while sending messages in a (background) periodic
task, then the
on_errorcallback is called, if set. If the callback isconfigured and returns
Truethen the task continues, otherwise it isaborted.
While it is possible to update a task with a callback after the task has
been created, this procedure is prone to a race condition where the
callback might not be configured in time for the first send event. Thus,
if the first send fails and the callback has not yet been configured,
the task will abort.
This commit solves the race condition issue by adding an argument to
BusABC.send_periodicto specify the callback. By including thecallback in the constructor it will be deterministically active for all
sends in the task. This fixes issue #1282.
This commit also adds myself to the CONTRIBUTORS list.