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: move the commands return value to bytes #1617

Merged
merged 34 commits into from
Jun 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
79b81d7
Moved to return bytes instead of string
GilboaAWS Jun 16, 2024
57cb7ad
check
Jun 16, 2024
21db3a7
Convert VerbatimString to binary
GilboaAWS Jun 17, 2024
c44afff
Moved Info command to support bytes.
GilboaAWS Jun 17, 2024
79bc635
match tests to bytes returns
Jun 17, 2024
88e90d7
* Fixed info and config get return values from bytes to str.
GilboaAWS Jun 18, 2024
5b87c62
Run black
GilboaAWS Jun 18, 2024
dddee6a
Moved more commands to work with bytes
GilboaAWS Jun 18, 2024
0498d5e
change tests to get bytes from commands
Jun 18, 2024
c76d75d
change tests to get bytes from commands
Jun 18, 2024
955ea8b
More tests fix for bytes
GilboaAWS Jun 18, 2024
0a44f67
Fix tests to bytes
GilboaAWS Jun 18, 2024
03657f9
change simpleString and transaction tests
Jun 19, 2024
490daf3
fix simpleString tests
Jun 20, 2024
caec88b
fix new tests to bytes
Jun 20, 2024
757839a
fix pr
Jun 23, 2024
ee4d9f1
revert core change
Jun 23, 2024
0989f74
change new commands from pull
Jun 23, 2024
e00b929
fix lint
Jun 23, 2024
a837810
fix lint
Jun 23, 2024
11d0ae4
fix mypy errors
Jun 26, 2024
2124914
change info command to return bytes
Jun 26, 2024
c05083e
merge main
Jun 26, 2024
a36e658
fix linter
Jun 26, 2024
4f3f14d
fix lint
Jun 26, 2024
ad415b9
fix new commits to bytes
Jun 27, 2024
7310846
fix new commands to bytes
Jun 27, 2024
f86c868
merge main and fix new commands
Jun 29, 2024
5666344
fix merge and tests
Jun 30, 2024
d8d262a
fix merge and new tests
Jun 30, 2024
bd429d0
fix lint
Jun 30, 2024
b18ff66
fix pr comments
Jun 30, 2024
8ce7aca
fix functions and tests
shohamazon Jun 30, 2024
2ca5e0d
Apply suggestions from code review
shohamazon Jun 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions examples/python/client_example.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ async def test_standalone_client(host: str = "localhost", port: int = 6379):
await send_set_and_get(client)
# Send PING to the primary node
pong = await client.custom_command(["PING"])
print(f"PONG response is = {pong}")
assert isinstance(pong, bytes)
print(f"PONG response is = {pong.decode()}")


async def test_cluster_client(host: str = "localhost", port: int = 6379):
Expand All @@ -71,10 +72,11 @@ async def test_cluster_client(host: str = "localhost", port: int = 6379):
await send_set_and_get(client)
# Send PING to all primaries (according to Redis's PING request_policy)
pong = await client.custom_command(["PING"])
print(f"PONG response is = {pong}")
assert isinstance(pong, bytes)
print(f"PONG response is = {pong.decode()}")
# Send INFO REPLICATION with routing option to all nodes
info_repl_resps = await client.custom_command(["INFO", "REPLICATION"], AllNodes())
print(f"INFO REPLICATION responses to all nodes are = {info_repl_resps}")
print(f"INFO REPLICATION responses from all nodes are = {info_repl_resps!r}")


async def main():
Expand Down
6 changes: 3 additions & 3 deletions python/python/glide/async_commands/cluster_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from __future__ import annotations

from typing import Dict, List, Mapping, Optional, cast
from typing import Dict, List, Mapping, Optional, Union, cast
Copy link
Collaborator

Choose a reason for hiding this comment

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

same


from glide.async_commands.command_args import Limit, OrderBy
from glide.async_commands.core import (
Expand Down Expand Up @@ -46,7 +46,7 @@ async def info(
self,
sections: Optional[List[InfoSection]] = None,
route: Optional[Route] = None,
) -> TClusterResponse[str]:
) -> TClusterResponse[bytes]:
adarovadya marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why bytes, but not Union[str, bytes] or even better TGlideString which is Union[str, bytes] (similar to TClusterResponse)?

"""
Get information and statistics about the Redis server.
See https://redis.io/commands/info/ for details.
Expand All @@ -65,7 +65,7 @@ async def info(
args = [section.value for section in sections] if sections else []

return cast(
TClusterResponse[str],
TClusterResponse[bytes],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Update line 70 please

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

await self._execute_command(RequestType.Info, args, route),
)

Expand Down
6 changes: 3 additions & 3 deletions python/python/glide/async_commands/standalone_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ async def custom_command(self, command_args: List[str]) -> TResult:
async def info(
self,
sections: Optional[List[InfoSection]] = None,
) -> str:
) -> bytes:
adarovadya marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Update lines 53 and 56. Doesn't mypy detect this?

"""
Get information and statistics about the Redis server.
See https://redis.io/commands/info/ for details.
Expand All @@ -49,10 +49,10 @@ async def info(


Returns:
str: Returns a string containing the information for the sections requested.
bytes: Returns bytes containing the information for the sections requested.
"""
args = [section.value for section in sections] if sections else []
return cast(str, await self._execute_command(RequestType.Info, args))
return cast(bytes, await self._execute_command(RequestType.Info, args))

async def exec(
self,
Expand Down
5 changes: 4 additions & 1 deletion python/python/glide/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@
float,
Set[T],
List[T],
bytes,
Dict[bytes, "TResult"],
Mapping[bytes, "TResult"],
]
TRequest = Union[RedisRequest, ConnectionRequest]
# When routing to a single node, response will be T
# Otherwise, response will be : {Address : response , ... } with type of Dict[str, T].
TClusterResponse = Union[T, Dict[str, T]]
TClusterResponse = Union[T, Dict[bytes, T]]
TSingleNodeRoute = Union[RandomNode, SlotKeyRoute, SlotIdRoute, ByAddressRoute]
# When specifying legacy path (path doesn't start with `$`), response will be T
# Otherwise, (when specifying JSONPath), response will be List[Optional[T]].
Expand Down
4 changes: 4 additions & 0 deletions python/python/glide/glide_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ async def _execute_command(
request = RedisRequest()
request.callback_idx = self._get_callback_index()
request.single_command.request_type = request_type
request.single_command.args_array.args[:] = [
bytes(elem, encoding="utf8") if isinstance(elem, str) else elem
for elem in args
]
(encoded_args, args_size) = self._encode_and_sum_size(args)
if args_size < MAX_REQUEST_ARGS_LEN:
request.single_command.args_array.args[:] = encoded_args
Expand Down
Loading
Loading