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

[Pavel Hrdina]

On Wed, Feb 06, 2019 at 03:56:37PM +0000, Daniel P. Berrangé wrote:
> 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.

I kind of agree with Andrea, the preferred formatting in python is to
use {} and .format() method, the '%' formatting is old and should not
be used.  In this case you have these options:

    "Can't load facts for '{}': {}".format(host, ex)

    "Can't load facts for '{0}': {1}".format(host, ex)

    "Can't load facts for '{host}': {ex}".format(host=host, ex=ex)

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190207/4c8e69a3/attachment-0001.sig>