From 5e523150ddf1aab058c097e11a323c1df4aaaee6 Mon Sep 17 00:00:00 2001 From: Nicolas Simonds Date: Thu, 9 May 2024 18:02:32 -0700 Subject: [PATCH] fix: RedisBroker: Remove non-idempotent jobs from running, too Adds a test that the running job is actually gone. Fixes: Issue #45 --- spinach/brokers/redis.py | 16 +++++++--------- tests/test_redis_brokers.py | 1 + 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/spinach/brokers/redis.py b/spinach/brokers/redis.py index 6fee72a..e3b1ef6 100644 --- a/spinach/brokers/redis.py +++ b/spinach/brokers/redis.py @@ -226,15 +226,13 @@ def get_jobs_from_queue(self, queue: str, max_jobs: int) -> List[Job]: return jobs def remove_job_from_running(self, job: Job): - if job.max_retries > 0: - self._run_script( - self._remove_job_from_running, - self._to_namespaced(RUNNING_JOBS_KEY.format(self._id)), - self._to_namespaced(MAX_CONCURRENCY_KEY), - self._to_namespaced(CURRENT_CONCURRENCY_KEY), - job.serialize(), - ) - + self._run_script( + self._remove_job_from_running, + self._to_namespaced(RUNNING_JOBS_KEY.format(self._id)), + self._to_namespaced(MAX_CONCURRENCY_KEY), + self._to_namespaced(CURRENT_CONCURRENCY_KEY), + job.serialize(), + ) self._something_happened.set() def _subscriber_func(self): diff --git a/tests/test_redis_brokers.py b/tests/test_redis_brokers.py index 330a5a8..6b31e92 100644 --- a/tests/test_redis_brokers.py +++ b/tests/test_redis_brokers.py @@ -86,6 +86,7 @@ def test_running_job(broker): ) # Try to remove it, even if it doesn't exist in running broker.remove_job_from_running(job) + assert broker._r.hget(running_jobs_key, str(job.id)) is None # Idempotent job - get from queue job = Job('foo_task', 'foo_queue', datetime.now(timezone.utc), 10)