[libvirt] [jenkins-ci PATCH v2 4/9] lcitool: include root cause when failing to load facts

Daniel P. Berrangé berrange at redhat.com
Wed Feb 6 15:56:37 UTC 2019


On Wed, Feb 06, 2019 at 04:53:46PM +0100, Andrea Bolognani wrote:
> On Tue, 2019-02-05 at 17:53 +0000, Daniel P. Berrangé wrote:
> [...]
> > @@ -202,8 +202,9 @@ class Inventory:
> >              try:
> >                  self._facts[host] = self._read_all_facts(host)
> >                  self._facts[host]["inventory_hostname"] = host
> > -            except Exception:
> > -                raise Error("Can't load facts for '{}'".format(host))
> > +            except Exception as ex:
> > +                raise Error("Can't load facts for '%(host)s': %(ex)s" %
> > +                            { "host": host, "ex": ex})
> 
> Did you actually run into a situation where this was useful? It's
> one of those diagnostics that I didn't really expect to trigger in
> practice...

Yes, if you create malformed yaml file this triggers. It means the
error message now tells you the place where the yaml syntax error is

> Either way, I don't really have a problem with adding it, but what
> I don't like is using a completely different way to format strings
> than the rest of the code.
> 
> I don't use Python nearly enough to have an opinion on the merits
> of either syntax compared to the other, so if the one you're using
> here is considered a best practice then we can definitely switch to
> it; however, it will have to be done in a separate commit that
> converts the entire script at once rather than in a piecemail
> fashion.

I've never come across the "{}" syntax before so didn't even
know how to use named params for it until now.

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 :|




More information about the libvir-list mailing list