ZADD thrashes on redis >= 3.0.2 with NX/CH/INCR/XX

[combatpoodle]

Issue

I had some issues with ZADD not actually adding values to the set if NX was set on redis > 3.0.2. When XX, NX, CH, or INCR was used they were interpreted to be the score and were then intermingled with extra pairs of arguments throughout the function.

Workaround

Just stripping out zadd's internals does get it working:

diff --git a/src/replay/src/lib/txredisapi.py b/src/replay/src/lib/txredisapi.py
index cf233d6..3557589 100644
--- a/src/replay/src/lib/txredisapi.py
+++ b/src/replay/src/lib/txredisapi.py
@@ -1118,11 +1118,22 @@ class BaseRedisProtocol(LineReceiver, policies.TimeoutMixin):
         return self.execute_command("SSCAN", key, *args)

     # Commands operating on sorted zsets (sorted sets)
+    def zadd(self, key, *args):
-    def zadd(self, key, score, member, *args):
         """
         Add the specified member to the Sorted Set value at key
         or update the score if it already exist
         """
-        if args:
-            # Args should be pairs (have even number of elements)
-            if len(args) % 2:
-                return defer.fail(InvalidData(
-                    "Invalid number of arguments to ZADD"))
-            else:
-                l = [score, member]
-                l.extend(args)
-                args = l
-        else:
-            args = [score, member]
         return self.execute_command("ZADD", key, *args)

     def zrem(self, key, *args):

Solution

By the previous behavior I'm guessing you want to keep the error checking in there - would you like it re-added and PR'd with a check for NX/CH/XX/INCR?

[IlyaSkriblovsky]

Good find, thanks!

Parsing "NX", "CH", ... from *args would be obscure and non-obvious API to my taste.

I'd prefer boolean kwargs: zadd("key", 2, "two", 3, "three", nx=True). Unfortunately, we can't have keyword args after *args in Python 2, but it can be simulated with **kwargs like this:

def zadd(self, key, score, member, *args, **kwargs):
    nx = kwargs.get('nx', False)
    ...

what do you think?

[fiorix]

👍