django-redis
django-redis copied to clipboard
jazzband
Default client does not pass `blocking` kwarg through lock method
Traceback:
lock = cache_redis_lock.lock(name, blocking=False, timeout=240)
File "/home/vagrant/.tox/py37/lib/python3.7/site-packages/django_redis/cache.py", line 31, in _decorator
return method(self, *args, **kwargs)
File "/home/vagrant/.tox/py37/lib/python3.7/site-packages/django_redis/cache.py", line 173, in lock
return self.client.lock(*args, **kwargs)
TypeError: lock() got an unexpected keyword argument 'blocking'
This should be an acceptable kwarg per https://github.com/redis/redis-py/blob/3.5.3/redis/lock.py#L74 and the threading.Lock implementation.
It is not listed in the lock signature on the default client at https://github.com/jazzband/django-redis/blob/5.0.0/django_redis/client/default.py#L329
Can you look again - it appears to be an error in the client lock() method. The redis-py lock allows the parameter. I checkd before submitting the issue and linked the link in the issue
DefaultClient.lock does not accept **kwargs and does not list blocking as an argument it accepts in the signature
I would add blocking argument along with *args and **kwargs to that function, would you be up for a PR?
Actually, if you are calling lock.acquire() then you can pass the blocking parameter.
The client.lock() in redis-py does not allow blocking parameter to be passed so the only way is to use lock.acquire().
We cannot use *args and **kwargs because they are not going to solve the problem
I would suggest to open an issue and maybe a PR in redis-py first and then make changes to django-redis, otherwise the desired behavior is not possible.
@WisdomPill that's not true: https://github.com/redis/redis-py/blob/master/redis/client.py#L1099
Therefore, I think your PR will solve it.
Hello @AqeelAT,
I actually made the pr not long ago to be able to proceed with #603 in redis/redis-py#2137 :) But unfortunately I did not get back to #603 just yet
Oh I just noticed that you're the one who made https://github.com/redis/redis-py/pull/2137. We could definitely benefit from #603 I think it is ready to be merged.
