On Thu, Feb 14, 2019 at 10:46:21AM +0100, Andrea Bolognani wrote:
> On Wed, 2019-02-13 at 19:03 +0000, Daniel P. Berrangé wrote:
> [...]
> > @@ -177,8 +177,8 @@ class Inventory:
> > parser = configparser.SafeConfigParser()
> > parser.read(ansible_cfg_path)
> > inventory_path = parser.get("defaults", "inventory")
> > - except Exception:
> > - raise Error("Can't find inventory location in ansible.cfg")
> > + except Exception as ex:
> > + raise Error("Can't read inventory location in ansible.cfg: {}".format(ex))
>
> flake8 complains about this line being too long, so please rewrite
> it as
What args (if any) are you giving to flake8 when you test this, as if we
want patches to pass flake8 tests we should add a makefile with a
'check' target. This would imply we should the make check in our CI
system. Testing the test system is rather nicely recursive :-)
>
> raise Error(
> "Can't read inventory location in ansible.cfg: {}".format(ex)
> )
>
> to make it happy.
>
> [...]
> > @@ -273,8 +273,8 @@ class Projects:
> > with open(yaml_path, "r") as infile:
> > packages = yaml.load(infile)
> > self._packages[project] = packages["packages"]
> > - except Exception:
> > - raise Error("Can't load packages for '{}'".format(project))
> > + except Exception as ex:
> > + raise Error("Can't load packages for '{}': {}".format(project, ex))
>
> This line is also too long and needs to be reformatted.
>
> [...]
> > @@ -398,8 +398,8 @@ class Application:
> >
> > try:
> > subprocess.check_call(cmd)
> > - except Exception:
> > - raise Error("Failed to run {} on '{}'".format(playbook, hosts))
> > + except Exception as ex:
> > + raise Error("Failed to run {} on '{}': {}".format(playbook, hosts), ex)
>
> 'ex' should be an argument to format(), not Error(). This kind of
> stuff is exactly why Python is so much fun! :P
>
> You also need to reformat it to prevent flake8 from complaining
> about its length.
>
>
> With those issues addressed,
>
> Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
