Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python: Added STRLEN command in python #1230

Merged
merged 11 commits into from
Apr 8, 2024
Merged

Conversation

GilboaAWS
Copy link
Collaborator

No description provided.

@GilboaAWS GilboaAWS added the python Python wrapper label Apr 4, 2024
@GilboaAWS GilboaAWS requested a review from a team as a code owner April 4, 2024 09:56
@GilboaAWS GilboaAWS force-pushed the main branch 2 times, most recently from 0372ee7 to 27f8d54 Compare April 4, 2024 10:26

Returns:
int: The length of the string value stored at key.
If `key` does not exist, it is interpreted as an empty list and 0 is returned.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If `key` does not exist, it is interpreted as an empty list and 0 is returned.
If `key` does not exist, it is interpreted as an empty string and 0 is returned.


Commands response:
int: The length of the string value stored at key.
If `key` does not exist, it is interpreted as an empty list and 0 is returned.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If `key` does not exist, it is interpreted as an empty list and 0 is returned.
If `key` does not exist, it is interpreted as an empty string and 0 is returned.

assert await redis_client.lpush(key1, value_list) == 4
with pytest.raises(RequestError) as e:
assert await redis_client.strlen(key1)
assert "Operation against a key holding the wrong kind of value" in str(e)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep in mind that different redis version return different error message

@@ -1042,6 +1042,24 @@ async def llen(self, key: str) -> int:
"""
return cast(int, await self._execute_command(RequestType.LLen, [key]))

async def strlen(self, key: str) -> int:
"""
Get the length of the string value stored at key.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Get the length of the string value stored at key.
Get the length of the string value stored at `key`.

async def strlen(self, key: str) -> int:
"""
Get the length of the string value stored at key.
See https://redis.io/commands/strlen/ for details.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
See https://redis.io/commands/strlen/ for details.
See https://redis.io/commands/strlen/ for more details.

See https://redis.io/commands/strlen/ for details.

Args:
key (str): The key
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
key (str): The key
key (str): The key to return its length.

python/python/glide/async_commands/core.py Outdated Show resolved Hide resolved
python/python/glide/async_commands/core.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@ikolomi ikolomi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix @shohamazon comments and consider changing the test not to test for the exact error message per @Yury-Fridlyand

python/python/glide/async_commands/core.py Outdated Show resolved Hide resolved
@GilboaAWS GilboaAWS force-pushed the main branch 2 times, most recently from 5da2582 to 1090bf7 Compare April 7, 2024 12:25
python/python/glide/async_commands/transaction.py Outdated Show resolved Hide resolved
python/python/glide/async_commands/transaction.py Outdated Show resolved Hide resolved
python/python/glide/async_commands/core.py Outdated Show resolved Hide resolved
python/python/glide/async_commands/core.py Outdated Show resolved Hide resolved
python/python/tests/test_async_client.py Outdated Show resolved Hide resolved
python/python/tests/test_async_client.py Show resolved Hide resolved
python/python/tests/test_async_client.py Outdated Show resolved Hide resolved
@GilboaAWS GilboaAWS changed the title Added Strlen command in python Python: Added Strlen command in python Apr 7, 2024
@shohamazon shohamazon changed the title Python: Added Strlen command in python Python: Added STRLEN command in python Apr 7, 2024
@GilboaAWS GilboaAWS merged commit 10490fd into valkey-io:main Apr 8, 2024
6 checks passed
shohamazon pushed a commit to adanWattad/glide-for-redis that referenced this pull request Apr 9, 2024
* Python: adds STRLEN command
---------
Co-authored-by: Shoham Elias
cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 24, 2024
* Python: adds STRLEN command
---------
Co-authored-by: Shoham Elias
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Python wrapper
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants