Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default client does not pass blocking kwarg through lock method #596

Closed
thenewguy opened this issue Mar 30, 2022 · 13 comments · Fixed by #752
Closed

Default client does not pass blocking kwarg through lock method #596

thenewguy opened this issue Mar 30, 2022 · 13 comments · Fixed by #752

Comments

@thenewguy
Copy link

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

@thenewguy thenewguy added the bug label Mar 30, 2022
@WisdomPill
Copy link
Member

That is a redis-py issue which should be taken care of there.

@thenewguy
Copy link
Author

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

@thenewguy
Copy link
Author

@WisdomPill ^

@thenewguy
Copy link
Author

DefaultClient.lock does not accept **kwargs and does not list blocking as an argument it accepts in the signature

@WisdomPill
Copy link
Member

sorry, I thought you were using lock as context manager

@WisdomPill WisdomPill reopened this Mar 30, 2022
@WisdomPill
Copy link
Member

WisdomPill commented Mar 30, 2022

I would add blocking argument along with *args and **kwargs to that function, would you be up for a PR?

@WisdomPill
Copy link
Member

WisdomPill commented Apr 18, 2022

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

@WisdomPill
Copy link
Member

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.

@aqeelat
Copy link
Member

aqeelat commented Jun 28, 2022

@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.

@WisdomPill
Copy link
Member

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

@aqeelat
Copy link
Member

aqeelat commented Jun 28, 2022

Oh I just noticed that you're the one who made redis/redis-py#2137.
We could definitely benefit from #603
I think it is ready to be merged.

@tevariou
Copy link

tevariou commented Oct 2, 2024

no update on this ?

@WisdomPill
Copy link
Member

Done, sorry for the long time needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants