-
Notifications
You must be signed in to change notification settings - Fork 11
Add server version validation during module loading #48
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
Conversation
Status::Ok | ||
fn initialize(ctx: &Context, _args: &[ValkeyString]) -> Status { | ||
let ver = ctx | ||
.get_redis_version() |
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.
We can have a small change in the valkeymodule-rs crate to rename this to get_server_version
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.
PR to valkeymodule-rs: valkey-io/valkeymodule-rs#178. Awaiting for the decision
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.
Nice! Thank you
e44ac48
to
6c03bea
Compare
Signed-off-by: VanessaTang <[email protected]>
6c03bea
to
c53cfbc
Compare
Check server version during module loading. If server version not satisfy minimal required version, module failed loading with message in log.
Testing
Manually Testing
Created 2 versions of valkey-server and valkey-cli:
make
valkey/unstable locally withversion.h
updated toVALKEY_VERSION 7.255.255
andVALKEY_VERSION_NUM 0x0007FFFF
.make
valkey/unstableLoad module during server initialized
./valkey-server-7255255 --loadmodule /<path>/libvalkey_bloom.so
Sever aborting.
Output like following:
Loading module via command module load
Client response with
(error) ERR Error loading the extension. Please check the server logs.
Server initialized with error message in log,
Integration Test
./build.sh
successfully completed. All tests passed.