MinoMino / minqlx

Extends Quake Live's dedicated server with extra functionality and scripting.
GNU General Public License v3.0
110 stars 42 forks source link

provisional try to make minqlx independent from redis v2 #115

Open mgaertne opened 1 year ago

mgaertne commented 1 year ago

This is a first try to make minqlx redis less reliant on redis v2 being installed. I tried to incorporate backward and forward compatibility as much as possible. I did not yet try this out, though. Basic information point were the information from redis v3 page: https://pypi.org/project/redis/3.0.0/ I ignored SETEX, LREM, TTL, and PTTL changes. Not sure whether any plugin uses these features.

em92 commented 1 year ago

From discord:

[15:48] ShiN0: the order of arguments in zincrby has been changed, instead of value, amount it's amount, value in redis >=3 (or the other way around). I checked for the second parameter filled in whether it's an int or float, and determined then whether it's a v2 call or a v3 call. Unfortunately if you call that function with a steam_id (it's an int) as value and 1 as amount, that logic might end up doing the wrong thing.

In this case, I suggest to make database class modifications, only if redis version is 3 or greater. For example:

class Redis(AbstractDatabase):
   # ...
   if redis >= (3, 0, 0):
     def zincrby(self, name, value, amount=1):
       return self.r.zincrby(name, amount, value)

     #def mset(blablabla):

In that case Database's API will be like redis-py v2's API.

mgaertne commented 1 year ago

I think it's more complicated than that. In my plugins, I incorporated some code that checks the redis version as well, and tries to call the functions in question in the right manner. That logic sort of collides with anything put into database.py.

em92 commented 1 year ago

In my plugins, I incorporated some code that checks the redis version as well, and tries to call the functions in question in the right manner.

Since you switched back to redis-py v2, maybe you will revert commits with redis version checks?

mgaertne commented 1 year ago

That's a possibility. However, I grow concerned when the redis-py documentation and minqlx's adaptation go too far apart from each other. Ideally, one could check whether value and amount are both numeric, and if so, pick the smaller of the two to be the amount in the resulting call. I'm not sure whether that heuristic would be good-enough.