Python: adds JSON.STRLEN, JSON.STRAPPEND commands #1187

shohamazon · 2024-03-27T11:07:31Z

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

python/python/glide/async_commands/redis_modules/json.py

shohamazon · 2024-03-31T08:32:00Z

python/python/glide/async_commands/redis_modules/json.py

+    Args:
+        client (TRedisClient): The Redis client to execute the command.
+        key (str): The key of the JSON document.
+        path (Optional[str]): The JSONPath to specify. Default is root `$`. // TODO check (since when sending strlen "non_exiting" and strlen "non_exiting" . -> behaves the same)


when running the commands:

127.0.0.1:6379> json.strlen non_existing_key (nil) 127.0.0.1:6379> json.strlen non_existing_key . (nil) 127.0.0.1:6379> json.strlen non_existing_key $ (error) ERR could not perform this operation on a key that doesn't exist

so I am not really sure that default it $

Then remove the comment to what it defaults to, just say in the documentation that if not passed a default value will be used

barshaul · 2024-04-24T08:29:41Z

CHANGELOG.md

@@ -52,6 +52,7 @@
 * Core: Enabled Cluster Mode periodic checks by default ([#1089](https://github.com/aws/glide-for-redis/pull/1089))
 * Node: Added Rename command. ([#1124](https://github.com/aws/glide-for-redis/pull/1124))
 * Python: Added JSON.TOGGLE command ([#1184](https://github.com/aws/glide-for-redis/pull/1184))
+* Python: Added JSON.STRLEN, JSON.STAPPEND commands ([#1187](https://github.com/aws/glide-for-redis/pull/1187))


move it up to the right version

barshaul · 2024-04-24T08:37:32Z

python/python/glide/async_commands/redis_modules/json.py

+        path (Optional[str]): The JSONPath to specify. If not passed, a default value will be used.
+
+    Returns:
+        TJsonResponse[Optional[int]]: For JSONPath (`path` starts with `$`), returns a list of integer replies for every possible path,


lets simplify it by splitting the return to two sections:

Returns: TJsonResponse[Optional[int]]: For JSONPath (`path` starts with `$`): Returns a list of integer replies for every possible path, indicating the length of the JSON string value, or None for JSON values matching the path that are not string. If `key` doesn't exist, an error is raised. For legacy path (`path` doesn't start with `$`): Returns the length of the JSON value at `path` or None if `key` doesn't exist. If the JSON value at `path` is not a string of if `path` doesn't exist, an error is raised, If `key` doesn't exist, None is returned.

please do it for all

barshaul · 2024-04-24T08:44:54Z

python/python/tests/tests_redis_modules/test_json.py

        with pytest.raises(RequestError):
            assert await json.toggle(redis_client, "non_exiting_key", "$")
+
+    @pytest.mark.parametrize("cluster_mode", [True, False])
+    @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3])


what about tests without specifying path?

barshaul · 2024-04-24T08:48:55Z

python/python/glide/async_commands/redis_modules/json.py

+    Args:
+        client (TRedisClient): The Redis client to execute the command.
+        key (str): The key of the JSON document.
+        path (Optional[str]): The JSONPath to specify. If not passed, a default value will be used.


lets try to stick with the document in ElastiCache docs, if it fits also for OSS
https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/json-strlen.html

shohamazon added the python label Mar 27, 2024

shohamazon requested a review from a team as a code owner March 27, 2024 11:07

Yury-Fridlyand approved these changes Mar 27, 2024

View reviewed changes

shohamazon changed the title ~~Python: adds JSON.STRLEN command~~ Python: adds JSON.STRLEN, JSON.STRAPPEND commands Mar 28, 2024

barshaul reviewed Mar 31, 2024

View reviewed changes