Ensure wakeup() is called even when add_job() raises an exception#1107
Closed
nuglifeleoji wants to merge 2 commits into
Closed
Ensure wakeup() is called even when add_job() raises an exception#1107nuglifeleoji wants to merge 2 commits into
nuglifeleoji wants to merge 2 commits into
Conversation
If the job store's add_job() call raised an unexpected exception (e.g. a SQLAlchemy error), _real_add_job() would propagate the exception before reaching the `self.wakeup()` call at the end of the method. This left the scheduler's event loop without a wakeup signal, preventing already-scheduled jobs from being triggered. Move wakeup() into a `finally` block so it is always called when the scheduler is in STATE_RUNNING, regardless of whether the add_job() call succeeded or failed. The job-added event and log message are still only emitted on success (they remain inside the try block). Fixes #1092 Made-with: Cursor
Owner
|
Why should the scheduler be woken up if adding the job failed? The whole premise seems odd to me. If the job store raises an exception from |
…tion" This reverts commit 774f661.
32bdcba to
34a46c2
Compare
Owner
|
Judging by the fact that you force-pushed a revert which makes this PR a no-op, I conclude that you were not serious about this. |
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.
Problem (fixes #1092)
If the job store's
add_job()call raises an unexpected exception (e.g. a SQLAlchemy error during a database write),_real_add_job()propagates the exception before reaching theself.wakeup()call at the bottom of the method.This leaves the scheduler's event loop without a wakeup signal. Already-scheduled, unrelated jobs stop being triggered until the scheduler is manually woken up.
Fix
Wrap the body of
_real_add_jobin atry/finallyso thatwakeup()is always called when the scheduler isSTATE_RUNNING, regardless of whetheradd_job()succeeded or failed.The job-added event dispatch and log message are left inside the
tryblock and are therefore only emitted on success — the semantics of those operations are unchanged.Made with Cursor