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

Redis to valkey? #882

Open
mlathara opened this issue Aug 5, 2024 · 12 comments · May be fixed by #975
Open

Redis to valkey? #882

mlathara opened this issue Aug 5, 2024 · 12 comments · May be fixed by #975

Comments

@mlathara
Copy link

mlathara commented Aug 5, 2024

I was wondering if there was any interest or ongoing work in migrating from redis to valkey? Or in adding valkey as a separate backend for this project?

@Dreamsorcerer
Copy link
Member

If someone wants to do the work, I think switching the existing redis backend to valkey would be fine.

@jules-ch
Copy link

jules-ch commented Nov 10, 2024

Monkeypatching should do the trick:

>>> import valkey.asyncio as redis
>>> redis.Redis = redis.Valkey # valkey does not ship with Redis in asyncio module
>>> from aiocache import BaseCache, Cache
>>> aiocache.backends.redis.redis = redis

>>> cache = Cache.from_url(settings.CACHE_URL)
<valkey.client.Valkey(<valkey.connection.ConnectionPool(<valkey.connection.Connection(host=127.0.0.1,port=6379,db=0)>)>)>

@Dreamsorcerer
Copy link
Member

If all you need is to change the import, should be simple to create a PR, rather than monkeypatching.

@jules-ch
Copy link

People might be interested in choosing their client either redis or valkey.
Creating a dedicated backend might be the way to do just that.

@amirreza8002
Copy link

if this is still of interest i'll do the work

the way i think would be good is to make a base class (let's call it key_value(as they are key-value databases))
then implement each client in a separate file (valkey and redis)

i think the tests can be re-used with some parametrizing as well

let me know what you think

@Dreamsorcerer
Copy link
Member

The valkey library should work fine with Redis still, so I see little point in maintaining both. By the time they start forking in incompatible ways I suspect very few people will be using Redis anymore. So, let's just proceed with switching to valkey.

One thing that might be worth looking at though is switching the implementation to valkey-glide, as it looks like that might have better performance.

@amirreza8002
Copy link

ok then
i'll dig around valkey-glide to see how we could make use of it

one question tho, will there only be a client for standalone valkey servers, or is there plan for cluster support as well?

@Dreamsorcerer
Copy link
Member

i'll dig around valkey-glide to see how we could make use of it

If it's too complex, feel free to stick with the valkey-py one, but would probably be nice to have.

one question tho, will there only be a client for standalone valkey servers, or is there plan for cluster support as well?

I have little familiarity with it, feel free to propose a solution for cluster support (ideally in a separate PR).

@asafpamzn
Copy link

I'm a valkey-glide maintainer. I will be happy to help. In our benchmarks it provides better performance and also more robust.

@amirreza8002
Copy link

hi @asafpamzn,
good to have you, i'll make a PR with my codes soon, it still has some problems
i'll continue to work on it, and appreciate any help to solve the issues it has

@amirreza8002 amirreza8002 linked a pull request Apr 1, 2025 that will close this issue
3 tasks
@Dreamsorcerer
Copy link
Member

@asafpamzn If you could do a review of #975, that'd be great. I've left a couple of comments, which I wonder if they are limitations of glide, so would be good to get your feedback on those.

@asafpamzn
Copy link

@Dreamsorcerer , I answered the questions, please review. In general, we will be a happy to enhance glide to meet the requirements of aiocache. Thus, if anything is needed don't hesitate to ask.

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

Successfully merging a pull request may close this issue.

5 participants