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

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.