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
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
