On Wed, May 13, 2020 at 07:00:49PM +0200, Andrea Bolognani wrote: > On Tue, 2020-05-12 at 16:42 +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 YAML 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 (ACKed) > > > > Since RFC: > > - replaced TOML with YAML which simplified some aspects of the code, thanks > > Andrea > > - instead of hardcoding the default values, the config within the repo is used > > as a template and overriden with user-selected options > > > > Since v1: > > - uncommented the mandatory options in the default template YAML config so that > > we know about all the supported keys which is useful for validating the user > > config > > - removed guests/group_vars/all/install.yaml in patch 11 (which I forgot in v1) > > - added checks for value types we get from the config > > - use yaml.safe_load instead of yaml.load > > - added code snippet to delete keys we don't recognize so as not to introduce a > > security issue, because we essentially just take the config and pass it to > > ansible, we don't want users to use to re-define some of Ansible's variables > > - added the last patch just to demonstrate a number of test cases I used as a > > proof for correctness of this revision (feel free to add more cases), but > > this is not the right series to add pytest support into lcitool > > You should have all the ACKs you need now. > > Thanks for being patient during review, and for taking care of this > in the first place :) Thanks for the review, changes are now merged. -- Erik Skultety
