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: adds JSON.TOGGLE command #1184

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

shohamazon
Copy link
Collaborator

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.

@shohamazon shohamazon requested a review from a team as a code owner March 26, 2024 15:50
CHANGELOG.md Outdated Show resolved Hide resolved
glide-core/src/client/value_conversion.rs Outdated Show resolved Hide resolved
python/python/tests/tests_redis_modules/test_json.py Outdated Show resolved Hide resolved
Co-authored-by: Yury-Fridlyand <[email protected]>
@@ -27,3 +27,7 @@
# Otherwise, response will be : {Address : response , ... } with type of Dict[str, T].
TClusterResponse = Union[T, Dict[str, 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]].
Copy link
Collaborator

Choose a reason for hiding this comment

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

(when specifying JSONPath) , => (when specifying JSONPath),
(remove redundant space)


Examples:
>>> from glide import json as redisJson
>>> await json.set(client, "doc", "$", json.dumps({"bool": true , "nested": {"bool": false , "nested": {"bool": 10}}}))
Copy link
Collaborator

@barshaul barshaul Mar 27, 2024

Choose a reason for hiding this comment

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

Suggested change
>>> await json.set(client, "doc", "$", json.dumps({"bool": true , "nested": {"bool": false , "nested": {"bool": 10}}}))
>>> await redisJson.set(client, "doc", "$", json.dumps({"bool": true , "nested": {"bool": false , "nested": {"bool": 10}}}))
"OK"

[False, True, None] # Indicates successful toggling of the Boolean values at path '$.bool' in the key stored at `doc`.
>>> await redisJson.toggle(client, "doc", "bool")
True # Indicates successful toggling of the Boolean value at path 'bool' in the key stored at `doc`.
>>> await redisJson.get(client, "doc", "$")
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
>>> await redisJson.get(client, "doc", "$")
>>> json.loads(await redisJson.get(client, "doc", "$"))

and then output it accordingly

path (str): The JSONPath to specify.

Returns:
TJsonResponse[bool]: Returns an array of boolean replies for each path, with the new value (false or true), or None for JSON values matching the path that are not Boolean.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"for each path" is confusing as 'path' is a single value

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can also refer to TJsonResponse to get more info


Returns:
TJsonResponse[bool]: Returns an array of boolean replies for each path, with the new value (false or true), or None for JSON values matching the path that are not Boolean.
If `path` doesn't start with `$` (legacy path syntax), returns the new value of the toggled value in `path`. Note that when sending legacy path syntax, the value at `path` must be a boolean value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that when sending legacy path syntax, the value at path must be a boolean value.

instead, write that if X then returns error

@@ -121,6 +122,46 @@ pub(crate) fn convert_to_expected_type(
ExpectedReturnType::BulkString => Ok(Value::BulkString(
from_owned_redis_value::<String>(value)?.into(),
)),
ExpectedReturnType::JsonToggleReturnType => match value {
Value::Array(array) => {
let new_array: Result<Vec<_>, _> = array
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
let new_array: Result<Vec<_>, _> = array
let new_array: RedisResult<Vec<_>> = array

Value::Array(array) => {
let new_array: Result<Vec<_>, _> = array
.into_iter()
.map(|arr| match arr {
Copy link
Collaborator

@barshaul barshaul Mar 27, 2024

Choose a reason for hiding this comment

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

arr -> item

Comment on lines 142 to 146

match new_array {
Ok(values) => Ok(Value::Array(values)),
Err(err) => Err(err),
}
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
match new_array {
Ok(values) => Ok(Value::Array(values)),
Err(err) => Err(err),
}
new_array.map(Value::Array)

@@ -121,6 +122,46 @@ pub(crate) fn convert_to_expected_type(
ExpectedReturnType::BulkString => Ok(Value::BulkString(
from_owned_redis_value::<String>(value)?.into(),
)),
ExpectedReturnType::JsonToggleReturnType => match value {
Value::Array(array) => {
let new_array: Result<Vec<_>, _> = array
Copy link
Collaborator

Choose a reason for hiding this comment

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

new_array -> converted_array

Err(err) => Err(err),
}
}
Value::BulkString(s) => match std::str::from_utf8(&s) {
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
Value::BulkString(s) => match std::str::from_utf8(&s) {
Value::BulkString(bytes) => match std::str::from_utf8(&bytes) {

@shohamazon shohamazon merged commit faca726 into valkey-io:main Mar 27, 2024
45 checks passed
@shohamazon shohamazon deleted the python/json.toggle branch March 27, 2024 10:36
shohamazon added a commit to adanWattad/glide-for-redis that referenced this pull request Apr 9, 2024
cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 24, 2024
@shohamazon shohamazon mentioned this pull request Oct 28, 2024
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants