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

Andrea Bolognani abologna at redhat.com
Wed Feb 6 15:53:46 UTC 2019


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

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.

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list