Re: [libvirt-ci PATCH v2 06/13] lcitool: Introduce methods to load and validate the YAML config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux