From ace8026c300947fa211b954837374d3b56408863 Mon Sep 17 00:00:00 2001 From: Nicolas Simonds Date: Sat, 30 Apr 2022 12:41:06 -0700 Subject: [PATCH] Make the Redis broker clean up aborted jobs The Redis broker uses a non-zero value for `max_retries` to determine if it should remove a job from the "running" state. So when an `AbortException` is thrown, max out the `job.retries` counter, rather than zeroing out the `job.max_retries` counter. This will have the same effect, but play more nicely with what the Redis broker is expecting. The MemoryBroker is unaffected by this. Fixes Issue #20 --- spinach/job.py | 2 +- tests/test_job.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/spinach/job.py b/spinach/job.py index 2136fc6..b0b183c 100644 --- a/spinach/job.py +++ b/spinach/job.py @@ -189,7 +189,7 @@ def advance_job_status(namespace: str, job: Job, duration: float, return if isinstance(err, AbortException): - job.max_retries = 0 + job.retries = job.max_retries logger.error( 'Fatal error during execution of %s after %s, canceling retries', job, duration, exc_info=err diff --git a/tests/test_job.py b/tests/test_job.py index 0c887be..592b28c 100644 --- a/tests/test_job.py +++ b/tests/test_job.py @@ -118,10 +118,13 @@ def test_advance_job_status(job): RetryException('Must retry', at=now)) assert job.status is JobStatus.FAILED + # The job should have retried twice due to previous tests, ensure that + # an AbortException tops off the retry counter job.status = JobStatus.RUNNING job.max_retries = 10 + assert job.retries == 2 advance_job_status('namespace', job, 1.0, AbortException('kaboom')) - assert job.max_retries == 0 + assert job.retries == 10 assert job.status is JobStatus.FAILED