-
Notifications
You must be signed in to change notification settings - Fork 53
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
How about support aioredis 2.0? #97
Comments
What do we gain by dropping v1.x support? Currently we have code to support all versions, and the only real thing we use is GET and EVALSHA and upload script, so we don't really even use anything complicated in aioredis. I am not a fan of preemptively removing support for older versions of libraries when they do not add significant time to development. If we got to a point where we needed something from aioredis 2.0 that was not also easily supported in 1.x, then i would consider dropping the older versions. |
OK, try compatibility support for aioredis 2.0 maybe better |
I can see that PR #99 includes support for v2.0.0. Any status on this? |
Just lack of time at the current moment. The last step before I start handling tests is to hammer out the connections for aioredis 2.0 with the goal of not forcing users to change their configurations. |
Thanks for the quick response! I understand. I can also see that |
Oh yeah, neat it does look like that lock does the same things. I may just set the dependencies to <2 and deprecate aioredlock for newer aioredis versions, I will have to think about that. |
I see, thank you. 😊 |
Hi @gtmanfred, It looks like the combo aioredis <2.0 + aioredlock doesn't work anymore on Python 3.10. Thank you! |
If someone else proposes a PR for support, I would merge and release, but I am not planning on finishing #99. |
Got it, thanks for the quick answer. |
The lock built in aioredis that I mention above works for us. |
Thanks for the feedback @JonasKs ❤️ |
I still have some question on this guys.
I don't understand your point that aioredis 2.0 locks does the same things as aioredlock.
What are the road blocks for PR #99? What do you think is missing? Can I help you finish it?
I'm curious about your use case. Were you using the Redlock algorithm for your locks before? Thanks! |
Yeah, and now we swapped to the lock used in aioredis v2. Whether it's completely equal or not, I don't really know. All our tests pass and we haven't had any issues though. Example on how it's implemented in an open source package by @sondrelg: https://github.com/sondrelg/asgi-idempotency-header/blob/main/idempotency_header_middleware/backends/aioredis.py#L64 |
Aioredlock is not equal to Aioredis locks because it provides guarantees about the lock and better fault tolerance than single Redis instance solutions since there are multiple independent Redis running. If an Aioredis lock is enough for you, then why did you chose Aioredlock in the first place? Aren't you afraid of loosing the robustness of Redlock algorithm? I assume you have only one running Redis instance, does it became a SPOF to you? Thanks for the link! |
Since Quoting my self:
|
Oh okay, I didn't knew that. It makes sense for your use case then.
Of course, I agree you didn't said that, my comment was in response to Daniel's comment:
I'm just trying to understand what is the best for someone willing to use the Redlock algorithm on Python 3.10+. Your use case is different, but thanks for the feedback anyway 👍 |
Hello, I'm also very interested in knowing if Aioredis 2.0 locks can be used in replacement of Aioredlock (for a 3-node Redis cluster). @gtmanfred how would you use them so the setup "does the same thing"? Thanks in advance! |
When i said it does the same thing, i meant it implemented the same lock mechanism on the redis instance. I have not double checked that it does a check against multiple redis instances, that would need to be implemented. Unfortunately I do not have the time right now to finish up the pr for aioredlock at the current moment as I am working on moving in December, but I can pick up the work again in January, and would also happily review, merge and release anyone that can get the work done. Thanks, |
I updated the requires to <2.0.0 for now, I have not had time to finish the pr, if anyone wants to take a shot at finishing it, that would be great. At some point i think it will be best to just drop <2.0.0 versions of aioredis, and do a major version bump and just maintain the 0. branch as a long term release, instead of supporting all aioredis versions. |
@gtmanfred I took a look at upgrading Here's the tested PR: #111 Keen to make a |
See https://aioredis.readthedocs.io/en/latest/migration/
I see the source code and found that support any version of aioredis, but if support aioredis 2.0 maybe drop that.
What do you think about?
The text was updated successfully, but these errors were encountered: