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
