On Tue, 2020-05-12 at 16:42 +0200, Erik Skultety wrote:
> + def _validate_section(self, config, section, mandatory_keys):
> + # remove keys we don't recognize
> + self._remove_unknown_keys(config[section], self.values[section].keys())
> +
> + # check that the mandatory keys are present and non-empty
> + for key in mandatory_keys:
> + if config.get(section).get(key, None) is None:
Isn't
foo.get("bar")
defined as returning None if the "bar" key is not defined for the
dictionary foo? Basically, I think you could write this as
if config.get(section).get(key) is None:
> + def _validate(self, config):
> + # delete sections we don't recognize
> + self._remove_unknown_keys(config, self.values.keys())
> +
> + if "install" not in config:
> + raise Exception("Missing mandatory section 'install'")
> +
> + self._validate_section(config, "install", ["root_password"])
> +
> + # we only need this for the gitlab check below, if 'flavor' is missing
> + # that's okay, we'll provide a default later
> + flavor = config["install"].get("flavor", None)
Same here...
> + def _update(self, values):
> + self.values["install"].update(values["install"])
> +
> + if values.get("gitlab", None):
... and here. In this specific case, I think the pythonic way to
write this check would be
if values.get("gitlab") is not None:
With these nits addressed,
Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
