This series is trying to consolidate the number of config files we currently
recognize under ~/.config/lcitool to a single global TOML config file. Thanks
to this effort we can expose more seetings than we previously could which will
come handy in terms of generating cloudinit images for OpenStack.

Patches 1-4 patches are just a little extra - not heavily related to the series
See patch 5/13 why TOML was selected.

Erik Skultety (13):
  requirements: Introduce a requirements.txt file
  lcitool: Decrease the indent when creating a tempdir for initrd
    injection
  lcitool: Prefer tempfile's native wrappers over low level primitives
  lcitool: Use a temporary JSON file to pass extra variables
  config: Introduce a new global config.toml configuration file
  lcitool: Introduce methods to load and validate the TOML config
  lcitool: Drop the get_flavor() method
  lcitool: Drop the get_root_password_file() method
  lcitool: Drop the gitlab-related getter methods
  lcitool: Use d.update() on extra_vars for options coming from the
    config
  config: Move the virt-install settings from install.yml to the config
  lcitool: Use the config install options rather than install facts
  guests: README: Document the existence and usage of config.toml

 config.toml                              |  43 +++++
 guests/README.markdown                   |  19 +-
 guests/lcitool                           | 235 +++++++++++------------
 guests/playbooks/update/tasks/gitlab.yml |   2 -
 guests/playbooks/update/tasks/users.yml  |   2 +-
 requirements.txt                         |   3 +
 6 files changed, 175 insertions(+), 129 deletions(-)
 create mode 100644 config.toml
 create mode 100644 requirements.txt

-- 
2.25.3

On Wed, 2020-04-22 at 15:28 +0200, Erik Skultety wrote:
> This series is trying to consolidate the number of config files we currently
> recognize under ~/.config/lcitool to a single global TOML config file. Thanks
> to this effort we can expose more seetings than we previously could which will
> come handy in terms of generating cloudinit images for OpenStack.
> 
> Patches 1-4 patches are just a little extra - not heavily related to the series
> See patch 5/13 why TOML was selected.

First of all, thanks for tackling this! It's something that we've
sorely needed for a while now.

Now, I know I'm the one who suggested TOML in the first place... But
looking at the series now I can't help but think, why not YAML? O:-)

Since we're using it extensively already due to Ansible, I think it
would make sense to use it for the configuration file as well. It's
easy enough to consume for a human, and we wouldn't need to drag in
an additional dependency. I also believe, perhaps naively, that
adapting your code to use YAML instead of TOML wouldn't require much
work - from the Python point of view, you basically end up with a
dictionary in both cases.

Feel free to argue against this suggestion! For example, if you agree
with it in principle but feel like it's unfair of me to ask you to
rework your code, I'm happy to port it myself.

As for the rest of the series - I've only skimmed it so far, but I
did not spot anything horribly wrong with it at first glance. I'll
provide an actual, detailed review next week.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Thu, Apr 23, 2020 at 06:46:09PM +0200, Andrea Bolognani wrote:
> On Wed, 2020-04-22 at 15:28 +0200, Erik Skultety wrote:
> > This series is trying to consolidate the number of config files we currently
> > recognize under ~/.config/lcitool to a single global TOML config file. Thanks
> > to this effort we can expose more seetings than we previously could which will
> > come handy in terms of generating cloudinit images for OpenStack.
> > 
> > Patches 1-4 patches are just a little extra - not heavily related to the series
> > See patch 5/13 why TOML was selected.
> 
> First of all, thanks for tackling this! It's something that we've
> sorely needed for a while now.
> 
> Now, I know I'm the one who suggested TOML in the first place... But
> looking at the series now I can't help but think, why not YAML? O:-)

To be honest, even before you originally mentioned TOML, I myself had INI in
mind, so then I thought, yeah, why not go with TOML then, it's similar and more
powerful.
I did some comparison of several formats, because like you say, with YAML we'd
be more close to Ansible and I was on the cusp of choosing between YAML and
TOML and I felt like TOML was still more readable and expressive in terms of
simple configuration (and that's what Linux users are IMO used to from INI).
I was never a big fan of YAML really and when the dictionaries and list happen
to intertwine and nest a lot, YAML looses its readability quite quickly IMO,
which I never felt with TOML, but it's fair to say that my TOML experience is
very limited. That said, I don't expect us to have such a massive config, so
that multiple levels of YAML nesting will be necessary :).

> 
> Since we're using it extensively already due to Ansible, I think it
> would make sense to use it for the configuration file as well. It's
> easy enough to consume for a human, and we wouldn't need to drag in
> an additional dependency. I also believe, perhaps naively, that
> adapting your code to use YAML instead of TOML wouldn't require much
> work - from the Python point of view, you basically end up with a
> dictionary in both cases.
> 
> Feel free to argue against this suggestion! For example, if you agree
> with it in principle but feel like it's unfair of me to ask you to
> rework your code, I'm happy to port it myself.

I'd still prefer TOML, but I don't really have a compelling reason to argue
against YAML other than readability which I already admitted to be just a
matter of taste. Now on a more serious note, I'll wait for your detailed review
and then rework it in YAML in vX.

> 
> As for the rest of the series - I've only skimmed it so far, but I
> did not spot anything horribly wrong with it at first glance. I'll
> provide an actual, detailed review next week.

Okay, thanks.

-- 
Erik Skultety

On Fri, 2020-04-24 at 08:47 +0200, Erik Skultety wrote:
> On Thu, Apr 23, 2020 at 06:46:09PM +0200, Andrea Bolognani wrote:
> > Now, I know I'm the one who suggested TOML in the first place... But
> > looking at the series now I can't help but think, why not YAML? O:-)
> 
> To be honest, even before you originally mentioned TOML, I myself had INI in
> mind, so then I thought, yeah, why not go with TOML then, it's similar and more
> powerful.
> I did some comparison of several formats, because like you say, with YAML we'd
> be more close to Ansible and I was on the cusp of choosing between YAML and
> TOML and I felt like TOML was still more readable and expressive in terms of
> simple configuration (and that's what Linux users are IMO used to from INI).

Are you sure you didn't mean s/Linux/Windows/ here? ;)

> I'd still prefer TOML, but I don't really have a compelling reason to argue
> against YAML other than readability which I already admitted to be just a
> matter of taste. Now on a more serious note, I'll wait for your detailed review
> and then rework it in YAML in vX.

Eh, you know what, whatever. YAML is fine, TOML is fine, bringing
in an additional Python module is not a big deal. Let's fix actual
issues (if there are any) and skip at least some of the bikeshedding.

-- 
Andrea Bolognani / Red Hat / Virtualization