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

Provide Modules with the ability to reject CONFIG SETs on module configurations #123

Open
KarthikSubbarao opened this issue Oct 31, 2024 · 4 comments

Comments

@KarthikSubbarao
Copy link
Member

Today, Modules using valkeymodule-rs can create configs using the valkeymodule macro.

Modules can provide a callback that gets triggered on configuration change (after CONFIG SET).

But we do not have a way for Modules to reject CONFIG SET operations on Module configurations.

In Core valkey, we can do this by having the SDK accept a config_set callback, just like we allow modules to provide a on_configuration_change call back. If the Module returns an Error, the config set operation should be rejected. Else, it can be applied.

Current Usage:

    configurations: [
.....
        string: [
            ["string", &*CONFIGURATION_STRING, "default", ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed::<String, _>))],
        ],
......
@KarthikSubbarao
Copy link
Member Author

Hello - I am currently implementing a change for this issue and will push out a PR soon

@dmitrypol
Copy link
Collaborator

@KarthikSubbarao - so the use case is to prevent user from EVER changing special config that was defined in module? Or to restrict it config changes to certain values (like ENUM)? Or only enable during configs to be set during start like raw::REDISMODULE_CONFIG_IMMUTABLE? I am trying to understand the use case

@KarthikSubbarao
Copy link
Member Author

KarthikSubbarao commented Dec 2, 2024

This is meant as a handler for Mutable configs. When they are changed using CONFIG SET, we can benefit from a function that can be executed to decide whether the config should be rejected or not.

One example is I have a string config called error-rate:

        string: [
            ["error-rate", &*CONFIGURATION_STRING, "default", ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed::<String, _>))],
        ],

As part of my validation, I would like to do two things:

  1. Check that the string is a valid f64
  2. Check that it is in the range of 0.0 to 1.0

If these conditions do not apply, I would like to reject the config set.

Similarly, we can have this validation for other config types (integer, enum, bool), etc

@KarthikSubbarao
Copy link
Member Author

@dmitrypol - I have implemented this functionality here: #144

You can take a look when you get a chance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants