Re: [libvirt-ci PATCH v2 00/13] Introduce a new global YAML config file for lcitool

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux