[libvirt] [jenkins-ci PATCH 01/22] lcitool: Generate the unattended script file

[Andrea Bolognani]

On Mon, 2019-12-09 at 16:20 +0100, Fabiano Fidêncio wrote:
> Let's always generate the unattended script file as, by doing this, we
> can trea the files we have as templates, being able to change them

s/trea/treat/

> accordingly to whatever is needed by specific distros.
> 
> It's important to note that we *must* be careful and keep generating
> those files using their "expected" filename, preferrably using the same

s/preferrably/preferably/

[...]
> @@ -537,7 +539,14 @@ class Application:
>                  raise Exception(
>                      "Host {} doesn't support installation".format(host)
>                  )

It would be nice if you added a short comment here, explaining the
fact that we're generating the file on the fly starting from a
template.

> -            initrd_inject = os.path.join(base, "configs", install_config)
> +            initrd_template = os.path.join(base, "configs", install_config)
> +            with open(initrd_template) as template:

Missing "r" here. It's probably the default, but we're passing it
explicitly to open() everywhere else so let's be consistent.

> +                content = template.read()
> +
> +                tempdir = tempfile.mkdtemp()
> +                initrd_inject = os.path.join(tempdir, install_config)
> +
> +                open(initrd_inject, "w").write(content)

Shouldn't this also use a 'with' clause to ensure the file is closed
correctly after writing to it?

> @@ -587,6 +596,8 @@ class Application:
>              except Exception as ex:
>                  raise Exception("Failed to install '{}': {}".format(host, ex))
>  
> +            shutil.rmtree(tempdir, ignore_errors=True)

So if we error out at any point, the temporary directory will be left
behind... Eh, I can live with that :)

-- 
Andrea Bolognani / Red Hat / Virtualization