Lua binary support #1729
Merged
Lua binary support #1729
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
glide-core changes 1. Change InvocationScript message to use bytes instead of strings 2. Fix socket_listener and scripts_container to use byte array for storing scripts 3. update redis commands load, eval and the internal invoke_script to use byte array 4. small refactor in scripts_container java changes 1. Add new script options class for glide string script parameters 2. Fix storeScript ffi according to new message to get bytes array 3. Add new handler handleBinaryObjectOrNullResponse 4. Add Script constructore to get GlideString for script argument 5. Add Script class new member binarySafeOutput to be able to diffrintiate between output types on invocation
added 2 commits
June 30, 2024 15:53
There was a problem hiding this comment.
Replace existing options without introducing new classes
There was a problem hiding this comment.
Not fully understand
There was a problem hiding this comment.
Patch existing class to store GlideString instead of String
1. Change script constructor for to accept PyAny. No noverloads in rust for the constructor and python don't support templates 2. Chage _execute_script to convert keys,args to bytes
1. Change Script consturctor to accept string or uint8array 2. Change ScriptOptions to have args,keys of string or uint8array
eifrah-aws
requested changes
Jul 1, 2024
barshaul
reviewed
Jul 1, 2024
Comment on lines
4974
to
4975
| keys (Union[str, bytes]): The keys that are used in the script. | ||
| args (Union[str, bytes]): The arguments for the script. |
There was a problem hiding this comment.
Suggested change
| keys (Union[str, bytes]): The keys that are used in the script. | |
| args (Union[str, bytes]): The arguments for the script. | |
| keys (List[Union[str, bytes]]): The keys that are used in the script. | |
| args (List[Union[str, bytes]]): The arguments for the script. |
eifrah-aws
approved these changes
Jul 1, 2024
| @@ -658,7 +660,8 @@ CompletableFuture<Boolean> pexpireAt( | |||
| * @return a value that depends on the script that was executed. | |||
| * @example | |||
| * <pre>{@code | |||
| * try(Script luaScript = new Script("return { KEYS[1], ARGV[1] }")) { | |||
| * try(Script luaScript = new Script("return { KEYS[1], ARGV[1] }", true)) { | |||
| * luaScript.setBinarySafeOutput(true); | |||
There was a problem hiding this comment.
Suggested change
| * luaScript.setBinarySafeOutput(true); |
| @@ -635,7 +636,8 @@ CompletableFuture<Boolean> pexpireAt( | |||
| * @return a value that depends on the script that was executed. | |||
| * @example | |||
| * <pre>{@code | |||
| * try(Script luaScript = new Script("return 'Hello'")) { | |||
| * try(Script luaScript = new Script("return 'Hello'", true)) { | |||
| * luaScript.setBinarySafeOutput(true); | |||
There was a problem hiding this comment.
Suggested change
| * luaScript.setBinarySafeOutput(true); |
cyip10
pushed a commit
to Bit-Quill/valkey-glide
that referenced
this pull request
Jul 16, 2024
* Add support for GlideString for Lua scripts glide-core changes 1. Change InvocationScript message to use bytes instead of strings 2. Fix socket_listener and scripts_container to use byte array for storing scripts 3. update redis commands load, eval and the internal invoke_script to use byte array 4. small refactor in scripts_container java changes 1. Add new script options class for glide string script parameters 2. Fix storeScript ffi according to new message to get bytes array 3. Add new handler handleBinaryObjectOrNullResponse 4. Add Script constructore to get GlideString for script argument 5. Add Script class new member binarySafeOutput to be able to diffrintiate between output types on invocation python changes 1. Change script constructor for to accept PyAny. No noverloads in rust for the constructor and python don't support templates 2. Chage _execute_script to convert keys,args to bytes node changes 1. Change Script consturctor to accept string or uint8array 2. Change ScriptOptions to have args,keys of string or uint8array --------- Co-authored-by: Yulazari <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
Add support for GlideString for Lua scripts
glide-core changes
java changes
python changes
node changes
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.