[libvirt] [jenkins-ci PATCH v3 05/10] lcitool: include root cause when reporting errors

[Daniel P. Berrangé]

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 at redhat.com>
> 
> -- 
> 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 :|