[libvirt-ci PATCH 07/13] lcitool: Drop the get_flavor() method

Erik Skultety posted 13 patches 5 years, 9 months ago
[libvirt-ci PATCH 07/13] lcitool: Drop the get_flavor() method
Posted by Erik Skultety 5 years, 9 months ago
We can now access this value directly in the config dictionary.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 guests/lcitool | 32 +++-----------------------------
 1 file changed, 3 insertions(+), 29 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index f08bd98..a385c15 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -206,37 +206,12 @@ class Config:
         if flavor == "gitlab":
             self._validate_section(self.dict, "gitlab", "gitlab_runner_secret")
 
-    def get_flavor(self):
-        flavor_file = self._get_config_file("flavor")
-
-        try:
-            with open(flavor_file, "r") as infile:
-                flavor = infile.readline().strip()
-        except Exception:
-            # If the flavor has not been configured, we choose the default
-            # and store it on disk to ensure consistent behavior from now on
-            flavor = "test"
-            try:
-                with open(flavor_file, "w") as infile:
-                    infile.write("{}\n".format(flavor))
-            except Exception as ex:
-                raise Exception(
-                    "Can't write flavor file ({}): {}".format(
-                        flavor_file, ex
-                    )
-                )
-
-        if flavor not in ["test", "jenkins", "gitlab"]:
-            raise Exception("Invalid flavor '{}'".format(flavor))
-
-        return flavor
-
     def get_vault_password_file(self):
         vault_pass_file = None
 
         # The vault password is only needed for the jenkins flavor, but in
         # that case we want to make sure there's *something* in there
-        if self.get_flavor() == "jenkins":
+        if self.dict["install"]["flavor"] == "jenkins":
             vault_pass_file = self._get_config_file("vault-password")
 
             try:
@@ -535,7 +510,6 @@ class Application:
     def _execute_playbook(self, playbook, hosts, projects, git_revision):
         base = Util.get_base()
 
-        flavor = self._config.get_flavor()
         vault_pass_file = self._config.get_vault_password_file()
         root_pass_file = self._config.get_root_password_file()
         gitlab_url_file = self._config.get_gitlab_url_file()
@@ -568,7 +542,7 @@ class Application:
                 "base": base,
                 "playbook_base": playbook_base,
                 "root_password_file": root_pass_file,
-                "flavor": flavor,
+                "flavor": self._config.dict["install"]["flavor"],
                 "selected_projects": selected_projects,
                 "git_remote": git_remote,
                 "git_branch": git_branch,
@@ -617,7 +591,7 @@ class Application:
     def _action_install(self, args):
         base = Util.get_base()
 
-        flavor = self._config.get_flavor()
+        flavor = self._config.dict["install"]["flavor"]
 
         for host in self._inventory.expand_pattern(args.hosts):
             facts = self._inventory.get_facts(host)
-- 
2.25.3

Re: [libvirt-ci PATCH 07/13] lcitool: Drop the get_flavor() method
Posted by Andrea Bolognani 5 years, 9 months ago
On Wed, 2020-04-22 at 15:28 +0200, Erik Skultety wrote:
> @@ -617,7 +591,7 @@ class Application:
>      def _action_install(self, args):
>          base = Util.get_base()
>  
> -        flavor = self._config.get_flavor()
> +        flavor = self._config.dict["install"]["flavor"]

You can remove this assignment and just use the value directly
below, like you've done in _execute_playbook().

There are also two uses of get_flavor() remaining after this
patch:

  get_gitlab_runner_token_file()
  get_gitlab_url_file()

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt-ci PATCH 07/13] lcitool: Drop the get_flavor() method
Posted by Erik Skultety 5 years, 9 months ago
On Tue, Apr 28, 2020 at 09:34:54AM +0200, Andrea Bolognani wrote:
> On Wed, 2020-04-22 at 15:28 +0200, Erik Skultety wrote:
> > @@ -617,7 +591,7 @@ class Application:
> >      def _action_install(self, args):
> >          base = Util.get_base()
> >  
> > -        flavor = self._config.get_flavor()
> > +        flavor = self._config.dict["install"]["flavor"]
> 
> You can remove this assignment and just use the value directly
> below, like you've done in _execute_playbook().

Okay, missed that one.

> 
> There are also two uses of get_flavor() remaining after this
> patch:
> 
>   get_gitlab_runner_token_file()
>   get_gitlab_url_file()

Yeah, does it matter since I'm dropping those 2 patches later?

-- 
Erik Skultety

Re: [libvirt-ci PATCH 07/13] lcitool: Drop the get_flavor() method
Posted by Andrea Bolognani 5 years, 9 months ago
On Tue, 2020-04-28 at 10:20 +0200, Erik Skultety wrote:
> On Tue, Apr 28, 2020 at 09:34:54AM +0200, Andrea Bolognani wrote:
> > There are also two uses of get_flavor() remaining after this
> > patch:
> > 
> >   get_gitlab_runner_token_file()
> >   get_gitlab_url_file()
> 
> Yeah, does it matter since I'm dropping those 2 patches later?

In the grand scheme of things? No, not really :) But removing all
calls to a function when you remove the function itself is just the
right thing to do.

-- 
Andrea Bolognani / Red Hat / Virtualization