fixed PR comments

Author: DvirDukhan

docs/commands.md

@@ -583,8 +583,8 @@ A `TIMEOUT t` argument can be specified to cause a request to be removed from th
 
 ```
 AI.SCRIPTEXECUTE <key> <function> 
-KEYS n <key> [keys...] 
-[INPUTS m <input> [input ...]
+[KEYS n <key> [keys...]]
+[INPUTS m <input> [input ...]]
 [ARGS k <arg> [arg...]]
 [OUTPUTS k <output> [output ...] [TIMEOUT t]]+
 ```
@@ -593,13 +593,16 @@ _Arguments_
 
 * **key**: the script's key name
 * **function**: the name of the function to run
-* **KEYS**: Either a squence of key names that the script will access before, during and after its execution, or a tag which all those keys share. `KEYS` is a mandatory scope in this command. Redis will verify that all potional key accesses are done to the right shard.
+* **KEYS**: Either a squence of key names that the script will access before, during and after its execution, or a tag which all those keys share. 
 * **INPUTS**: Denotes the beginning of the input parameters list, followed by its length and one or more input tensors.
 * **ARGS**: A list additional arguments that a user can send to the script. All args are sent as strings, but can be casted to other types supported by torch script, such as `int`, or `float`. 
 
 * **OUTPUTS**: denotes the beginning of the output tensors keys' list, followed by its length and one or more key names.
 * **TIMEOUT**: the time (in ms) after which the client is unblocked and a `TIMEDOUT` string is returned
 
+Note:
+Either `KEYS` or `INPUTS` scopes should be provided this command (one or both scopes are acceptable). Those scopes indicate keyspace access and such, the right shard to execute the command at. Redis will verify that all potional key accesses are done to the right shard.
+
 _Return_
 
 A simple 'OK' string, a simple `TIMEDOUT` string, or an error.
@@ -832,7 +835,7 @@ A `TIMEOUT t` argument can be specified to cause a request to be removed from th
 ```
 AI.DAGEXECUTE [[LOAD <n> <key-1> <key-2> ... <key-n>] |
           [PERSIST <n> <key-1> <key-2> ... <key-n>] |
-          [ROUTING <routing_tag>]]+
+          [ROUTING <routing_tag>]]
           [TIMEOUT t]
           |> <command> [|>  command ...]
 ```

tests/flow/test_data/redis_scripts.py

@@ -26,6 +26,15 @@ def test_redis_error(tensors: List[Tensor], keys: List[str], args: List[str]):
     key = keys[0]
     redis.execute("SET", key)
 
+def test_redis_command_error(tensors: List[Tensor], keys: List[str], args: List[str]):
+    key = keys[0]
+    redis.execute("SET", key)  # missing the required value argument
+
+def test_redis_error_message(tensors: List[Tensor], keys: List[str], args: List[str]):
+    key = keys[0]
+    redis.execute("HSET", key, "field", "value")
+    redis.execute("RPUSH", key, "some_value")  # list command on a hash type
+
 def test_int_set_get(tensors: List[Tensor], keys: List[str], args: List[str]):
     key = keys[0]
     value = args[0]

tests/flow/test_torchscript_extensions.py

@@ -21,7 +21,21 @@ def __init__(self):
         self.con = self.env.getConnection()
         script = load_file_content('redis_scripts.py')
         ret = self.con.execute_command(
-            'AI.SCRIPTSTORE', 'redis_scripts{1}', DEVICE, 'ENTRY_POINTS', 11, 'test_redis_error', 'test_set_key', 'test_int_set_get', 'test_int_set_incr', 'test_float_set_get', 'test_int_list', 'test_str_list', 'test_hash', 'test_model_execute', 'test_model_execute_onnx', 'test_model_execute_onnx_bad_input', 'SOURCE', script)
+            'AI.SCRIPTSTORE', 'redis_scripts{1}', DEVICE, 'ENTRY_POINTS', 13, 
+            'test_redis_error',
+            'test_set_key', 
+            'test_int_set_get', 
+            'test_int_set_incr', 
+            'test_float_set_get', 
+            'test_int_list', 
+            'test_str_list', 
+            'test_hash', 
+            'test_model_execute', 
+            'test_model_execute_onnx', 
+            'test_model_execute_onnx_bad_input',
+            'test_redis_command_error',
+            'test_redis_error_message',
+            'SOURCE', script)
         self.env.assertEqual(ret, b'OK')
         model_tf = load_file_content('graph.pb')
         ret = self.con.execute_command('AI.MODELSTORE', 'model_tf{1}', 'TF', DEVICE, 'INPUTS', 2, 'a', 'b', 'OUTPUTS', 1,
@@ -36,12 +50,12 @@ def __init__(self):
         ensureSlaveSynced(self.con, self.env)
 
     def test_redis_error(self):
-        try:
-            self.con.execute_command(
-            'AI.SCRIPTEXECUTE', 'redis_scripts', 'test_redis_error', 'KEYS', 1, "x{1}")
-            self.env.assertTrue(False)
-        except:
-            pass
+        check_error_message(self.env, self.con, "Redis command returned an error: Invalid argument",
+                            'AI.SCRIPTEXECUTE', 'redis_scripts{1}', 'test_redis_command_error',  'KEYS', 1, "x{1}", error_msg_is_substr=True)
+        check_error_message(self.env, self.con, "Redis command returned an error: "
+                                                "WRONGTYPE Operation against a key holding the wrong kind of value",
+                            'AI.SCRIPTEXECUTE', 'redis_scripts{1}', 'test_redis_error_message',  'KEYS', 1, "hash{1}",  error_msg_is_substr=True)
+
 
     def test_simple_test_set(self):
         self.con.execute_command(

tests/flow/tests_dag_errors.py

@@ -235,4 +235,4 @@ def test_dag_crossslot_violation_errors(env):
             'INPUTS', 1, 'transactionTensor:{1}',
             'OUTPUTS', 1, 'resultTensor:{1}',
         )
-        check_error_message(env, con, "ROUTING value specified in the command hash to slot which does not belong to the current shard", *command)
+        check_error_message(env, con, "CROSSSLOT Keys in request don't hash to the same slot", *command)