The problem/use-case that the feature addresses
The current Redis module API allows having a module info command handler. This handler only receives an argument of whether the info should be sent back for the crash report or not. However, the user may also request specific sections to be printed out, and the module isn't aware of these sections having been requested. Currently, a way to solve this is a trial-and-error circle of constant attempts to send all the sections possible and ignore the failures. The failure to add a section means that the user didn't request it. The developer experience could be improved here by passing an array of section names to the module so that the module does only the job it is supposed to do instead of doing the extra job and ignoring the failures.
Description of the feature
I suggest passing an additional argument of an array of strings of the section names the user requested or a pair of nullptr and 0 (the length of the array) in case nothing specific was requested by the user.
Alternatives you've considered
The alternative is to leave the current implementation as is, but I am not considering it as an ultimately good developer experience.
Additional information
The API of the registering an info handler looks like this
REDISMODULE_API int (*RedisModule_RegisterInfoFunc)(RedisModuleCtx *ctx, RedisModuleInfoFunc cb) REDISMODULE_ATTR;
The callback is defined as this:
/* Function pointers needed by both the core and modules, these needs to be
* exposed since you can't cast a function pointer to (void *). */
typedef void (*RedisModuleInfoFunc)(RedisModuleInfoCtx *ctx, int for_crash_report);
The suggestion is to pass an array of section names, for example (there are probably more optimal ways):
typedef void (*RedisModuleInfoFunc)(RedisModuleInfoCtx *ctx, int for_crash_report, char **section_names, size_t section_names_length);
Comment From: oranagra
i don't think i understand the problem or the advantage in your proposal. AFAICT it just means move the string compare from one place to the other (and increase the chance the module will mess it up ,note that the conditions are a bit tricky).
Even with the current API, the module can try to add a section, and only collect the necessary information in case that API call succeeded. it seems quite straight forward to use IMHO (not a complicated trial and error as you suggested). maybe i don't understand your use case...
Comment From: iddm
It is not about the string comparing at all; it is about asking a module to do its job properly rather than making it guess what was the user input. The implementation we have now does not allow a module to simply "answer" with a proper section the user requested: the module code has to attempt to send as many sections as it can and get rejected (the AddSection will return RedisModuleErr) the user didn't want and successfully make a call to AddSection when it happens so that the section attempted to be added was the one user wanted. This is a guessing game. My suggestion is to remove the guessing process and ask a module to reply only with specific sections a user requested instead. This will also remove the extra job of sending any other section(s) the user didn't want and which we don't know the user didn't want. "Hey, module X, give me your sections Y and Z, as the user has just requested those", instead of "Hey, module X, give me all the sections you can, and I will be sending you errors when I don't need some of them".
Comment From: oranagra
i still don't see the disadvantage in the current approach, any extra work the module needs to do in order to fill a certain section can be done after adding the section header and skipped if it errors. if anything, it is more streamlined for the module to write code that uses the current interface than what you describe.
please note that the filtering of sections is not just X Y and Z and you say, there are also some implicit rules, e.g. in case of INFO EVERYTHING, INFO MODULES, or INFO MODULENAME.
In any case, the current API cannot be easily changed due to backwards compatibility, so unless there is a serious unsolvable issue, we're not gonna add a second alternative, which will just cause confusion.
Comment From: iddm
The confusion is already due to the proposed workflow of adding a section and its fields. Anyway, thank you for your time!
Comment From: oranagra
i meant that having to choose between two different APIs that achieve the same purpose is confusing (more documentation to read).
anyway, i still don't understand the benefits of your suggestion over the existing one. maybe if you submit a quick example of the same logic implemented in both APIs i'll get the point.