On 17/12/18 22:02, Sage Weil wrote: > See WIP pull request > > https://github.com/ceph/ceph/pull/25597 > > The idea is to better unify the module options and the 'ceph config ...' > commands. Namely, > > - (more) fully specify the module options, including type, description, > default, min/max, enum_allowed, and so on, just like the built-in config > options. > > - advertise these from mgr back to the mon via the MgrMap > > - expose these config options to the 'ceph config help' CLI +1 to all of the above > > There are some rough edges, though: > > - The current modules have a mix of conventions for the default values. > The MgrMap::ModuleOption makes everything a string (default value, min, > max, etc.) to keep the encoding simple, but half of the modules define > default values as string and case at time of use (e.g., via int(...)), > and half use the native types (e.g., values of None or 123). This makes the > generic code awkard. I think the main problem was that every option value was stored as a string and if we were not careful we could end up with a default value specified with a native int type, and then after setting a custom value we would get a string. I'm in favor of having the `mgr.get_module_option` to return a value of the type that was specified in the option descriptor. This fixes the problem of the default value type, because it does not matter if the default value was set as "1", when we get the value with `mgr.get_module_option`, it will convert the value into an `int`. > > - The mon code has two paths, one for the built-in Option and one for > ModuleOption. I'm not sure yet if it makes sure to unify these or not. > > - Option from common/options.h has a print method that is used for 'config > help ...'. The ModuleOption type implements its own print() that > has output similar to the above, but is a different > implementation. Why not convert the ModuleOption into an Option when the the mons receive the MgrMap? That way only one implementation is needed. -- Ricardo Dias Senior Software Engineer - Storage Team SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
Attachment:
signature.asc
Description: OpenPGP digital signature
