-
Notifications
You must be signed in to change notification settings - Fork 53
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
Changes from 31 commits
79b81d7
57cb7ad
21db3a7
c44afff
79bc635
88e90d7
5b87c62
dddee6a
0498d5e
c76d75d
955ea8b
0a44f67
03657f9
490daf3
caec88b
757839a
ee4d9f1
0989f74
e00b929
a837810
11d0ae4
2124914
c05083e
a36e658
4f3f14d
ad415b9
7310846
f86c868
5666344
d8d262a
bd429d0
b18ff66
8ce7aca
2ca5e0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
from glide.async_commands.command_args import Limit, OrderBy | ||
from glide.async_commands.core import ( | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why |
||
""" | ||
Get information and statistics about the Redis server. | ||
See https://redis.io/commands/info/ for details. | ||
|
@@ -65,7 +65,7 @@ async def info( | |
args = [section.value for section in sections] if sections else [] | ||
|
||
return cast( | ||
TClusterResponse[str], | ||
TClusterResponse[bytes], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update line 70 please There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
await self._execute_command(RequestType.Info, args, route), | ||
) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
_build_sort_args, | ||
) | ||
from glide.async_commands.transaction import BaseTransaction, Transaction | ||
from glide.async_commands.utils.utils import convert_bytes_to_string_dict | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same |
||
from glide.constants import OK, TOK, TResult | ||
from glide.protobuf.redis_request_pb2 import RequestType | ||
|
||
|
@@ -38,7 +39,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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update lines 53 and 56. Doesn't |
||
""" | ||
Get information and statistics about the Redis server. | ||
See https://redis.io/commands/info/ for details. | ||
|
@@ -52,7 +53,7 @@ async def info( | |
str: Returns a string containing the information for the sections requested. | ||
adarovadya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
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, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
from typing import Any, Dict, Mapping, Optional, Union | ||
|
||
from glide.constants import TClusterDecodedResponse, TClusterResponse | ||
adarovadya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
def convert_bytes_to_string_dict( | ||
adarovadya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# TODO: remove the str options | ||
byte_string_dict: Optional[ | ||
Union[Mapping[bytes, Any], Dict[bytes, Any], Mapping[str, Any], Dict[str, Any]] | ||
] | ||
) -> Optional[Dict[str, Any]]: | ||
""" | ||
Recursively convert the keys and values of a dictionary from byte strings to regular strings, | ||
handling nested dictionaries of any depth. | ||
|
||
Args: | ||
byte_string_dict (Optional[Union[Mapping[bytes, Any], Dict[bytes, Any]]]): | ||
A dictionary where keys and values can be byte strings or nested dictionaries. | ||
adarovadya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Returns: | ||
Optional[Dict[str, Any]]: | ||
A dictionary with keys and values converted to regular strings, or None if input is None. | ||
""" | ||
if byte_string_dict is None: | ||
return None | ||
|
||
def convert(item: Any) -> Any: | ||
if isinstance(item, dict): | ||
return {convert(key): convert(value) for key, value in item.items()} | ||
elif isinstance(item, bytes): | ||
return item.decode("utf-8") | ||
elif isinstance(item, list): | ||
adarovadya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return [convert(elem) for elem in item] | ||
adarovadya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
else: | ||
return item | ||
|
||
return convert(byte_string_dict) | ||
|
||
|
||
def convert_bytes_to_string_cluster_response( | ||
adarovadya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
cluster_response: Optional[TClusterResponse], | ||
) -> Optional[TClusterDecodedResponse]: | ||
""" | ||
Convert a TClusterResponse type with byte strings to a TClusterResponse type with regular strings, | ||
handling nested dictionaries of any depth. | ||
|
||
Args: | ||
cluster_response (Optional[Union[T, Dict[bytes, T]]]): | ||
adarovadya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
A cluster response which can be of type T or a dictionary with byte string keys and values. | ||
|
||
Returns: | ||
Optional[TClusterResponse]: | ||
A cluster response with all byte strings converted to regular strings. | ||
""" | ||
if cluster_response is None: | ||
return None | ||
|
||
if isinstance(cluster_response, dict): | ||
return convert_bytes_to_string_dict(cluster_response) | ||
|
||
return cluster_response |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,11 +22,15 @@ | |
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]] | ||
TClusterDecodedResponse = Union[T, Dict[str, T]] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is going to change/break the API everywhere... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the next PR, the API signatures will be changed
adarovadya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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]]. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same