add RESP3 new APIs #280
Merged
add RESP3 new APIs #280
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
Closed
Open
| .with_context(|| "failed to run string.set")?; | ||
|
|
||
| res.sort(); | ||
| assert_eq!(&res, &["a", "b", "b", "c", "d", "e"]); |
There was a problem hiding this comment.
I was wondering how this test pass but then I notice that its not, also this client does not really check the reply is resp3 right?
iddm
reviewed
Mar 25, 2023
|
@vityafx thanks for the review. I either addressed the comments or added more comments. Let me know what you think. |
iddm
reviewed
Mar 27, 2023
iddm
reviewed
Apr 4, 2023
|
@vityafx fixed the Debug implementation and also added Display. Let me know what you think. |
iddm
reviewed
Apr 4, 2023
iddm
reviewed
Apr 4, 2023
iddm
reviewed
Apr 4, 2023
iddm
previously approved these changes
Apr 5, 2023
iddm
approved these changes
Apr 5, 2023
MeirShpilraien
added a commit
that referenced
this pull request
Apr 5, 2023
Main changes: * Key space notification API will get the keys as `&[u8]` instead of `&str` because key name can be binary. * ErrorReply was changed to be an enum of `RedisError` or general message so we can return this error type even without having an actual `RedisModuleCallReply` error. * Introduce `DetachContext` struct which implements `Sync` and `Send`. This can be used to create a global context for logging only. * `replicate` can accept also binary data and not only utf8. * `create_string` can accept binary data and not only utf8. * `authenticate_user` implementation was fixed to use the correct `RedisModuleAPI`. * `ThreadSafeContext` lock function was fixed to avoid reuse the context and avoid duplicate context free. * `RedisModule_Init` was split to `RedisModule_Init` and `RedisModule_InitAPI` so we can initialise the API without register a module. This is usefull for modules that load more plugins and want to initialise the `RedisModuleAPI` on the plugin but without Register it as a another module. We should consider back-port this change to Redis. * Move `init_func` callback after finish registration of commands and configuration so configuration value will be applied when called. * Introduce `safe_clone` for `RedisModuleString`. In general `RedisModuleString` is none atomic ref counted object. So it is not safe to clone it if Redis GIL is not hold. `safe_clone` gets a context reference which indicating that Redis GIL is held. * Implement serde serialise and de-serialise for `RedisString` * Fix configuration API `module_args_as_configuration` option to work correctly on imutable configurations. * Added option to specify module costume config get and set commands. Depends on: #280
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.
The PR adds Resp3 support for both, return result from module command and getting result as resp3 on
ctx.call_ext(when resp3 option is used).Tests was added to verify both new functionalities.
The PR also refactor the
CallReplyAPI introduced on #290. Instead of having 2 types ofCallReplywe now have one type such that the root call reply created with'staticlifetime indicating that the user can hold it for as long as it want. InternalCallReplycan outlive its father.As part of the refactoring, the
CallReplywas changed to be more rust friendly.CallReplywas renamed toCallResult = Result<CallReply, ErrorCallReply>,CallReplyis an enum which allows to check theCallReplytype using amatchstatement.