Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 guests/lcitool | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index 35a6b68..5c2b785 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -550,18 +550,14 @@ class Application:
 
         temp = {}
 
+        keys = ["default", package_format, os_name, os_full]
         # We need to add the base project manually here: the standard
         # machinery hides it because it's an implementation detail
         for project in projects + ["base"]:
             for package in self._projects.get_packages(project):
-                if "default" in mappings[package]:
-                    temp[package] = mappings[package]["default"]
-                if package_format in mappings[package]:
-                    temp[package] = mappings[package][package_format]
-                if os_name in mappings[package]:
-                    temp[package] = mappings[package][os_name]
-                if os_full in mappings[package]:
-                    temp[package] = mappings[package][os_full]
+                for key in keys:
+                    if key in mappings[package]:
+                        temp[package] = mappings[package][key]
 
         pkgs = []
         for item in temp:
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, 2019-02-21 at 16:33 +0000, Daniel P. Berrangé wrote:
[...]
> +        keys = ["default", package_format, os_name, os_full]
>          # We need to add the base project manually here: the standard
>          # machinery hides it because it's an implementation detail
>          for project in projects + ["base"]:
>              for package in self._projects.get_packages(project):
> -                if "default" in mappings[package]:
> -                    temp[package] = mappings[package]["default"]
> -                if package_format in mappings[package]:
> -                    temp[package] = mappings[package][package_format]
> -                if os_name in mappings[package]:
> -                    temp[package] = mappings[package][os_name]
> -                if os_full in mappings[package]:
> -                    temp[package] = mappings[package][os_full]
> +                for key in keys:
> +                    if key in mappings[package]:
> +                        temp[package] = mappings[package][key]

Little historical note: the reason why the code looked like this in
the first place[1] is that its structure was supposed to mirror that
of playbooks/update/tasks/packages.yml as closely as possible - the
idea being that, if the Ansible implementation was correct, then the
Python one would most likely be as well.

Of course that's no longer the case as of dcded110e102, so it makes
perfect sense to go further down this road and make the code more
compact.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>


[1] In addition to Python being admittedly not my forte :)
-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list