-
Notifications
You must be signed in to change notification settings - Fork 666
Add parameter for on_error to BusABC.send_periodic#1283
Open
andebjor wants to merge 1 commit intohardbyte:main from
Open
Add parameter for on_error to BusABC.send_periodic #1283andebjor wants to merge 1 commit intohardbyte:main from
on_error to BusABC.send_periodic #1283andebjor wants to merge 1 commit intohardbyte:main from
Conversation
Codecov Report
Merging #1283 (a0e1f92) into develop (af55b0a) will not change coverage.
The diff coverage is100.00%.
@@ 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.
@andebjor
andebjor
force-pushed
the
periodic_callback_arg
branch
from
March 16, 2022 08:18
3b8bb3c to
a0e1f92
Compare
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will fail for subclasses that reimplement _send_periodic_internal()
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.