mgr/dashboard: fix None value for ints and floats#35521
Conversation
7c88c53 to
7857a29
Compare
There was a problem hiding this comment.
IMO this is only a workaround and as such the comments should be added. To finally fix this the corresponding modules must set a correct value. None is Python specific and not a value that can be set by a user via CLI for example. If a type is set to int then it should be not None but something like -1 or 0 or any value that represents a Nonein the real world and the specific behavior of the configuration option.
| for name in ['default_value', 'min', 'max']: | ||
| if option[name]: # Skip empty entries | ||
| if option[name] == 'None': # This is Python None | ||
| option[name] = None |
There was a problem hiding this comment.
| option[name] = None | |
| # Note, this is a workaround, to finally fix this | |
| # the module must set a correct value. | |
| option[name] = None |
There was a problem hiding this comment.
I think in this specific case, the dashboard should be forgiving here. if we want to assert the type, we have to do it in mgr_module.get_module_option
| for name in ['default_value', 'min', 'max']: | ||
| if option[name]: # Skip empty entries | ||
| if option[name] == 'None': # This is Python None | ||
| option[name] = None |
There was a problem hiding this comment.
| option[name] = None | |
| # Note, this is a workaround, to finally fix this | |
| # the module must set a correct value. | |
| option[name] = None |
|
pylint failed with |
da3126b to
bfcb93f
Compare
Fixes: https://tracker.ceph.com/issues/45963 Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
fixes: `R0912: Too many branches (14/12) (too-many-branches)` Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
bfcb93f to
4215ad5
Compare
Signed-off-by: Sebastian Wagner sebastian.wagner@suse.com
Fixes: https://tracker.ceph.com/issues/45963
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard backendjenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox