Catch slack errors and add a command to remove jobs #388
Conversation
This file contains 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
rebkwok
force-pushed
the
remove-job-and-catch-slack-errors
branch
from
a27ffde
to
5082977
Compare
evansd
approved these changes
Jan 8, 2024
| ) | ||
| return resp.data | ||
| except Exception as err: | ||
| attempts += 1 |
There was a problem hiding this comment.
Is is worth a small sleep() here? Might not be worth it, just wondered if you'd considered it.
There was a problem hiding this comment.
I did consider it; I guess for a maximum 3 sec delay it's probably worth adding
rebkwok
force-pushed
the
remove-job-and-catch-slack-errors
branch
from
5082977
to
abff127
Compare
If some unexpected error is encountered during the running of a job, the job may be updated in the database to indicate that it's running, but never complete or fail. This prevents the job being retried, and any further attempts to run the job only update the job scheduled to run after the current one is done. This adds a command to remove specific jobs by id (identified by calling status first).
If the slack client errors in some unexpected way, it can cause a scheduled job to error without returning any success/fail status, which means the job gets stuck in the "running" state, even though it's only been reserved and isn't actually running. Now we retry a max of 3 times and log the error if we can't do the notification. This could mean that jobs run and never report back, if the slack client is more than transiently broken, but then the chances are that simple commands like help are also broken and at least the output of jobs should be accessible in the job logs.
rebkwok
force-pushed
the
remove-job-and-catch-slack-errors
branch
from
abff127
to
8b69286
Compare
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.
On Friday Rich tried to run an op command, but nothing happened
https://bennettoxford.slack.com/archives/CGF9TKZLG/p1704475634495869
@BennettBot statussuggested that this request to run a measures preview from 2nd Jan was still running, so the new request was just scheduled. Something apparently happened that meant the job didn't report that it either failed or succeeded, but I can't see far enough back in the old logs to see if anything was logged. In any case, the "running" job blocked any attempts to retry it, so this PR adds a command to remove a job by id in case this happens again.Although I can't see if it was the case for the 2nd Jan job, there was an instance of an uncaught error from slack
This seems to be an occasional transient thing, but if we happen to encounter it when notifying slack of starting or completing a job, we don't want it to prevent the job from being marked as complete, so now it retries up to 3 times and then logs the error.