redis-py icon indicating copy to clipboard operation
redis-py copied to clipboard

Make commands have a minimum supported redis version

Open WisdomPill opened this issue 3 years ago • 2 comments

Many commands are added since a specific version of redis, since this library supports redis from version 4 officially (?), some commands are available for newer version are available in the library but not in the server.

My idea is to fetch the version on the first connection used right after auth in on_connect or add a connection_callback using INFO server. After that "cache" (weird naming since we are talking about redis) the server version in a class variable in connection_pool.

After that add a decorator that could be used in every command or group of commands like the following @requires_server_version(5, 0). Finally check that the connection_pool redis version so that the command will not even run if the command is not supported.

@chayim what do you think about this?

WisdomPill avatar Apr 28 '22 19:04 WisdomPill

@WisdomPill So.. it's something I keep thinking about. We've solved this in test (see conftest.py) for this reason. The issue I have is that the connection is already overloaded, and in my opinion not the fastest. I generally dislike things that make redis slower - though I totally agree with why users would want this.

Want to talk through it? When looking at other database libraries, it's the server side (as opposed to the client side) that usually fails - so by not doing this we would continue to be in line with those. But again - I care most about the user experience (even at the expense of internally developing the module).

chayim avatar May 04 '22 07:05 chayim

We could do something pythonic, so ask for forgiveness instead of permission.

As I see here the error is pretty structured, so I would say to just add minimum version in commands as a decorator and if an exception (new exception, like CommandNotSupportedException thrown from the inside execute_command) then the error can be very precise and say which server version is needed.

I totally agree to not add further commands and not degrade performance even if it is done just in the first connection. You never know how people use the library 😄

WisdomPill avatar May 04 '22 13:05 WisdomPill

This issue is marked stale. It will be closed in 30 days if it is not updated.

github-actions[bot] avatar May 05 '23 00:05 github-actions[bot]

IMHO asking forgiveness i just trying it. I'd say if one really cares - try command commands. I.. don't think this is the right approach with a fire and forget database + language.

chayim avatar May 24 '23 12:05 chayim