Merge pull request #1052 from andymccurdy/error_defaults

Fail gracefully with a default return value when 0 keys are are provided to a command expecting at least 1 key

Author: andymccurdy

redis/client.py

@@ -26,6 +26,7 @@
 )
 
 SYM_EMPTY = b('')
+EMPTY_RESPONSE = 'EMPTY_RESPONSE'
 
 
 def list_or_args(keys, args):
@@ -757,7 +758,12 @@ def execute_command(self, *args, **options):
 
     def parse_response(self, connection, command_name, **options):
         "Parses a response from the Redis server"
-        response = connection.read_response()
+        try:
+            response = connection.read_response()
+        except ResponseError:
+            if EMPTY_RESPONSE in options:
+                return options[EMPTY_RESPONSE]
+            raise
         if command_name in self.response_callbacks:
             return self.response_callbacks[command_name](response, **options)
         return response
@@ -1120,7 +1126,10 @@ def mget(self, keys, *args):
         Returns a list of values ordered identically to ``keys``
         """
         args = list_or_args(keys, args)
-        return self.execute_command('MGET', *args)
+        options = {}
+        if not args:
+            options[EMPTY_RESPONSE] = []
+        return self.execute_command('MGET', *args, **options)
 
     def mset(self, *args, **kwargs):
         """
@@ -3214,7 +3223,8 @@ def pipeline_execute_command(self, *args, **options):
 
     def _execute_transaction(self, connection, commands, raise_on_error):
         cmds = chain([(('MULTI', ), {})], commands, [(('EXEC', ), {})])
-        all_cmds = connection.pack_commands([args for args, _ in cmds])
+        all_cmds = connection.pack_commands([args for args, options in cmds
+                                             if EMPTY_RESPONSE not in options])
         connection.send_packed_command(all_cmds)
         errors = []
 
@@ -3229,12 +3239,15 @@ def _execute_transaction(self, connection, commands, raise_on_error):
 
         # and all the other commands
         for i, command in enumerate(commands):
-            try:
-                self.parse_response(connection, '_')
-            except ResponseError:
-                ex = sys.exc_info()[1]
-                self.annotate_exception(ex, i + 1, command[0])
-                errors.append((i, ex))
+            if EMPTY_RESPONSE in command[1]:
+                errors.append((i, command[1][EMPTY_RESPONSE]))
+            else:
+                try:
+                    self.parse_response(connection, '_')
+                except ResponseError:
+                    ex = sys.exc_info()[1]
+                    self.annotate_exception(ex, i + 1, command[0])
+                    errors.append((i, ex))
 
         # parse the EXEC.
         try:

tests/test_commands.py

@@ -427,6 +427,7 @@ def test_keys(self, r):
         assert set(r.keys(pattern='test*')) == keys
 
     def test_mget(self, r):
+        assert r.mget([]) == []
         assert r.mget(['a', 'b']) == [None, None]
         r['a'] = '1'
         r['b'] = '2'

tests/test_pipeline.py

@@ -114,6 +114,34 @@ def test_exec_error_raised(self, r):
             assert pipe.set('z', 'zzz').execute() == [True]
             assert r['z'] == b('zzz')
 
+    def test_transaction_with_empty_error_command(self, r):
+        """
+        Commands with custom EMPTY_ERROR functionality return their default
+        values in the pipeline no matter the raise_on_error preference
+        """
+        for error_switch in (True, False):
+            with r.pipeline() as pipe:
+                pipe.set('a', 1).mget([]).set('c', 3)
+                result = pipe.execute(raise_on_error=error_switch)
+
+                assert result[0]
+                assert result[1] == []
+                assert result[2]
+
+    def test_pipeline_with_empty_error_command(self, r):
+        """
+        Commands with custom EMPTY_ERROR functionality return their default
+        values in the pipeline no matter the raise_on_error preference
+        """
+        for error_switch in (True, False):
+            with r.pipeline(transaction=False) as pipe:
+                pipe.set('a', 1).mget([]).set('c', 3)
+                result = pipe.execute(raise_on_error=error_switch)
+
+                assert result[0]
+                assert result[1] == []
+                assert result[2]
+
     def test_parse_error_raised(self, r):
         with r.pipeline() as pipe:
             # the zrem is invalid because we don't pass any keys to it