Repository: NicolasLM/spinach (language: python)

Issue:
Jobs without concurrency limits generate current_concurrency hash keys in redis
These should not really generate keys as there's no point. In addition, the counts associated with each are incremented forever so it's possible at some point an overflow could occur.


Discussion / hints:
Maybe try this. 

Modify the RedisBroker class in spinach to handle jobs without concurrency limits. Specifically, you can add a check in the _acquire_lock method to see if the job has a concurrency limit, and if not, skip generating the current_concurrency hash key and incrementing its count.

```
from spinach.brokers.redis import RedisBroker

class CustomRedisBroker(RedisBroker):
    def _acquire_lock(self, job):
        if job.concurrency_limit:
            super()._acquire_lock(job)
        else:
            self._increment_concurrency_count(job, 0)

```

In this implementation, we create a new class CustomRedisBroker that inherits from RedisBroker. We override the _acquire_lock method to add the check for concurrency limits and skip generating the current_concurrency hash key if the job doesn't have a limit. Instead, we call the _increment_concurrency_count method with a count of 0 to avoid incrementing the count indefinitely.

Fix patch:
diff --git a/spinach/brokers/redis_scripts/get_jobs_from_queue.lua b/spinach/brokers/redis_scripts/get_jobs_from_queue.lua
index d5959c0..27e5e49 100644
--- a/spinach/brokers/redis_scripts/get_jobs_from_queue.lua
+++ b/spinach/brokers/redis_scripts/get_jobs_from_queue.lua
@@ -33,7 +33,7 @@ repeat
         -- track the running job
         redis.call('hset', running_jobs_key, job["id"], job_json)
         -- If tracking concurrency, bump the current value.
-        if max_concurrency ~= -1 then
+        if max_concurrency ~= nil and max_concurrency ~= -1 then
             redis.call('hincrby', current_concurrency_key, job['task_name'], 1)
         end
 


Test patch:
diff --git a/tests/test_redis_brokers.py b/tests/test_redis_brokers.py
index f4a3410..330a5a8 100644
--- a/tests/test_redis_brokers.py
+++ b/tests/test_redis_brokers.py
@@ -187,6 +187,21 @@ def test_cant_exceed_max_concurrency(broker):
     assert json.loads(queued.decode())['id'] == str(job2.id)
 
 
+def test_does_not_set_concurrency_key_when_no_max_concurrency(broker):
+    job = Job(
+        CONCURRENT_TASK_NAME, 'foo_queue', datetime.now(timezone.utc), 1,
+        # kwargs help with debugging but are not part of the test.
+        task_kwargs=dict(name='job'),
+    )
+    broker.enqueue_jobs([job])
+    returned_jobs = broker.get_jobs_from_queue('foo_queue', 2)
+    assert returned_jobs[0].task_kwargs == dict(name='job')
+    current = broker._r.hget(
+        broker._to_namespaced(CURRENT_CONCURRENCY_KEY), CONCURRENT_TASK_NAME
+    )
+    assert current is None
+
+
 def test_get_jobs_from_queue_returns_all_requested(broker):
     # If a job is not returned because it was over concurrency limits,
     # make sure the number of jobs requested is filled from other jobs