The root cause exception contains the useful information about what
really failed during loading of some resource, or running of a
command.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
guests/lcitool | 54 +++++++++++++++++++++++++-------------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool
index 759eff6..bd32d1f 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -95,10 +95,10 @@ class Config:
if not os.path.exists(config_dir):
try:
os.mkdir(config_dir)
- except Exception:
+ except Exception as ex:
raise Error(
- "Can't create configuration directory ({})".format(
- config_dir,
+ "Can't create configuration directory ({}): {}".format(
+ config_dir, ex,
)
)
@@ -117,10 +117,10 @@ class Config:
try:
with open(flavor_file, "w") as infile:
infile.write("{}\n".format(flavor))
- except Exception:
+ except Exception as ex:
raise Error(
- "Can't write flavor file ({})".format(
- flavor_file,
+ "Can't write flavor file ({}): {}".format(
+ flavor_file, ex
)
)
@@ -141,10 +141,10 @@ class Config:
with open(vault_pass_file, "r") as infile:
if not infile.readline().strip():
raise ValueError
- except Exception:
+ except Exception as ex:
raise Error(
- "Missing or invalid vault password file ({})".format(
- vault_pass_file,
+ "Missing or invalid vault password file ({}): {}".format(
+ vault_pass_file, ex
)
)
@@ -157,10 +157,10 @@ class Config:
with open(root_pass_file, "r") as infile:
if not infile.readline().strip():
raise ValueError
- except Exception:
+ except Exception as ex:
raise Error(
- "Missing or invalid root password file ({})".format(
- root_pass_file,
+ "Missing or invalid root password file ({}): {}".format(
+ root_pass_file, ex
)
)
@@ -177,8 +177,8 @@ class Inventory:
parser = configparser.SafeConfigParser()
parser.read(ansible_cfg_path)
inventory_path = parser.get("defaults", "inventory")
- except Exception:
- raise Error("Can't find inventory location in ansible.cfg")
+ except Exception as ex:
+ raise Error("Can't read inventory location in ansible.cfg: {}".format(ex))
inventory_path = os.path.join(base, inventory_path)
@@ -191,10 +191,10 @@ class Inventory:
for line in infile:
host = line.strip()
self._facts[host] = {}
- except Exception:
+ except Exception as ex:
raise Error(
- "Missing or invalid inventory ({})".format(
- inventory_path,
+ "Missing or invalid inventory ({}): {}".format(
+ inventory_path, ex
)
)
@@ -202,8 +202,8 @@ 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 '{}': {}".format(host, ex))
@staticmethod
def _add_facts_from_file(facts, yaml_path):
@@ -254,8 +254,8 @@ class Projects:
with open(mappings_path, "r") as infile:
mappings = yaml.load(infile)
self._mappings = mappings["mappings"]
- except Exception:
- raise Error("Can't load mappings")
+ except Exception as ex:
+ raise Error("Can't load mappings: {}".format(ex))
source = os.path.join(base, "vars", "projects")
@@ -273,8 +273,8 @@ class Projects:
with open(yaml_path, "r") as infile:
packages = yaml.load(infile)
self._packages[project] = packages["packages"]
- except Exception:
- raise Error("Can't load packages for '{}'".format(project))
+ except Exception as ex:
+ raise Error("Can't load packages for '{}': {}".format(project, ex))
def expand_pattern(self, pattern):
projects = Util.expand_pattern(pattern, self._packages, "project")
@@ -398,8 +398,8 @@ class Application:
try:
subprocess.check_call(cmd)
- except Exception:
- raise Error("Failed to run {} on '{}'".format(playbook, hosts))
+ except Exception as ex:
+ raise Error("Failed to run {} on '{}': {}".format(playbook, hosts), ex)
def _action_hosts(self, _hosts, _projects, _revision):
for host in self._inventory.expand_pattern("all"):
@@ -478,8 +478,8 @@ class Application:
try:
subprocess.check_call(cmd)
- except Exception:
- raise Error("Failed to install '{}'".format(host))
+ except Exception as ex:
+ raise Error("Failed to install '{}': {}".format(host, ex))
def _action_update(self, hosts, projects, git_revision):
self._execute_playbook("update", hosts, projects, git_revision)
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-listOn Wed, 2019-02-13 at 19:03 +0000, Daniel P. Berrangé wrote:
[...]
> @@ -177,8 +177,8 @@ class Inventory:
> parser = configparser.SafeConfigParser()
> parser.read(ansible_cfg_path)
> inventory_path = parser.get("defaults", "inventory")
> - except Exception:
> - raise Error("Can't find inventory location in ansible.cfg")
> + except Exception as ex:
> + raise Error("Can't read inventory location in ansible.cfg: {}".format(ex))
flake8 complains about this line being too long, so please rewrite
it as
raise Error(
"Can't read inventory location in ansible.cfg: {}".format(ex)
)
to make it happy.
[...]
> @@ -273,8 +273,8 @@ class Projects:
> with open(yaml_path, "r") as infile:
> packages = yaml.load(infile)
> self._packages[project] = packages["packages"]
> - except Exception:
> - raise Error("Can't load packages for '{}'".format(project))
> + except Exception as ex:
> + raise Error("Can't load packages for '{}': {}".format(project, ex))
This line is also too long and needs to be reformatted.
[...]
> @@ -398,8 +398,8 @@ class Application:
>
> try:
> subprocess.check_call(cmd)
> - except Exception:
> - raise Error("Failed to run {} on '{}'".format(playbook, hosts))
> + except Exception as ex:
> + raise Error("Failed to run {} on '{}': {}".format(playbook, hosts), ex)
'ex' should be an argument to format(), not Error(). This kind of
stuff is exactly why Python is so much fun! :P
You also need to reformat it to prevent flake8 from complaining
about its length.
With those issues addressed,
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Feb 14, 2019 at 10:46:21AM +0100, Andrea Bolognani wrote:
> On Wed, 2019-02-13 at 19:03 +0000, Daniel P. Berrangé wrote:
> [...]
> > @@ -177,8 +177,8 @@ class Inventory:
> > parser = configparser.SafeConfigParser()
> > parser.read(ansible_cfg_path)
> > inventory_path = parser.get("defaults", "inventory")
> > - except Exception:
> > - raise Error("Can't find inventory location in ansible.cfg")
> > + except Exception as ex:
> > + raise Error("Can't read inventory location in ansible.cfg: {}".format(ex))
>
> flake8 complains about this line being too long, so please rewrite
> it as
What args (if any) are you giving to flake8 when you test this, as if we
want patches to pass flake8 tests we should add a makefile with a
'check' target. This would imply we should the make check in our CI
system. Testing the test system is rather nicely recursive :-)
>
> raise Error(
> "Can't read inventory location in ansible.cfg: {}".format(ex)
> )
>
> to make it happy.
>
> [...]
> > @@ -273,8 +273,8 @@ class Projects:
> > with open(yaml_path, "r") as infile:
> > packages = yaml.load(infile)
> > self._packages[project] = packages["packages"]
> > - except Exception:
> > - raise Error("Can't load packages for '{}'".format(project))
> > + except Exception as ex:
> > + raise Error("Can't load packages for '{}': {}".format(project, ex))
>
> This line is also too long and needs to be reformatted.
>
> [...]
> > @@ -398,8 +398,8 @@ class Application:
> >
> > try:
> > subprocess.check_call(cmd)
> > - except Exception:
> > - raise Error("Failed to run {} on '{}'".format(playbook, hosts))
> > + except Exception as ex:
> > + raise Error("Failed to run {} on '{}': {}".format(playbook, hosts), ex)
>
> 'ex' should be an argument to format(), not Error(). This kind of
> stuff is exactly why Python is so much fun! :P
>
> You also need to reformat it to prevent flake8 from complaining
> about its length.
>
>
> With those issues addressed,
>
> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
>
> --
> Andrea Bolognani / Red Hat / Virtualization
>
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 2019-02-14 at 09:59 +0000, Daniel P. Berrangé wrote: > On Thu, Feb 14, 2019 at 10:46:21AM +0100, Andrea Bolognani wrote: > > flake8 complains about this line being too long, so please rewrite > > it as > > What args (if any) are you giving to flake8 when you test this No arguments, just straight up 'flake8 lcitool'. > as if we > want patches to pass flake8 tests we should add a makefile with a > 'check' target. It should probably be 'syntax-check' rather than 'check'. > This would imply we should the make check in our CI > system. Testing the test system is rather nicely recursive :-) Yeah, that would be pretty neat :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.