valkey-io / valkey-py

Valkey Python client based on a fork of redis-py
MIT License
21 stars 2 forks source link

Provide support for redis:// and rediss:// #23

Open kjaymiller opened 5 days ago

kjaymiller commented 5 days ago

https://github.com/valkey-io/valkey-py/blob/d116aa6efa03877a0c37598139a556554ad47c13/valkey/asyncio/connection.py#L1025-L1074

This states that only valkey://, valkeys://, and unix but some providers like aiven are still using the rediss://.

Is there a reason that we aren't allowing the redis:// and rediss:// protocols or at least providing a deprecation warning if they are used?

This error gave me initial pause on if I could use the valkey client to test with new providers of Valkey.

kjaymiller commented 5 days ago

also happy to contribute a PR that adds redis:// and rediss://.

aiven-sal commented 5 days ago

Thanks for reporting this issue!

You can use valkey:// and valkeys:// in place of redis:// and rediss:// regardless of what your provider says you should be using. Valkey and Redis protocol are the same at the moment and, if they will ever change, valkey-py will only support the Valkey protocol.

But I agree that the transition will be easier if we supported redis:// and rediss:// as aliases of valkey:// and valkeys://, as long as the two protocols remain identical.

Of course PRs are always welcome.

kjaymiller commented 5 days ago

happy to contribute. I think the biggest question to add would be should a warning be issued when redis:// or rediss:// are used?

aiven-sal commented 4 days ago

I think that, at this point, it would be nice if people could swap redis-py with valkey-py without noticing any difference at all. So I wouldn't add a warning. At some point in the future we should definitely add a deprecation warning, but for now I'd like to make the transition as seamless as possible.