Re: [libvirt] [jenkins-ci PATCH v2 10/12] lcitool: Add projects information handling
On Thu, Jul 12, 2018 at 05:19:27PM +0200, Andrea Bolognani wrote: > The original tool's limited scope meant loadins this > information was not needed, but we're going to start > making use of it pretty soon. > > Signed-off-by: Andrea Bolognani > --- > guests/lcitool | 47 +++ > 1 file changed, 47 insertions(+) > > diff --git a/guests/lcitool b/guests/lcitool > index d82c36f..3bd5fa7 100755 > --- a/guests/lcitool > +++ b/guests/lcitool > @@ -233,11 +233,58 @@ class Inventory: > def get_facts(self, host): > return self._facts[host] > > +class Projects: > + > +def __init__(self): > +try: > +with open("./vars/mappings.yml", "r") as f: There is clear information where how to run the lcitool in the docs in some patches befor so the relative paths that are used everywhere in the code are not causing a problem. Though IMO, I think it's clearer to have a variable (config option, hardcoded, env variable or whatever you decide), storing the path of these files so that this code is not dependent on relative paths. WDYT? > +mappings = yaml.load(f) > +self._mappings = mappings["mappings"] > +except: > +raise Error("Can't load mappings") > + > +self._packages = {} > +source = "./vars/projects/" > +for item in os.listdir(source): > +yaml_path = os.path.join(source, item) > +if not os.path.isfile(yaml_path): > +continue > +if not yaml_path.endswith(".yml"): > +continue > + > +project = os.path.splitext(item)[0] > + > +try: > +with open(yaml_path, "r") as f: > +packages = yaml.load(f) > +self._packages[project] = packages["packages"] > +except: > +raise Error("Can't load packages for '{}'".format(project)) > + > +def expand_pattern(self, pattern): > +projects = Util.expand_pattern(pattern, self._packages, "project") > + > +# Some projects are internal implementation details and should > +# not be exposed to the user > +internal_projects = [ "base", "blacklist", "jenkins" ] > +for project in internal_projects: > +if project in projects: > +projects.remove(project) > + > +return projects > + > +def get_mappings(self): > +return self._mappings > + > +def get_packages(self, project): > +return self._packages[project] > + > class Application: > > def __init__(self): > self._config = Config() > self._inventory = Inventory() > +self._projects = Projects() > > self._parser = argparse.ArgumentParser( > conflict_handler = "resolve", > -- > 2.17.1 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 10/12] lcitool: Add projects information handling
On Tue, 2018-07-17 at 15:04 +0200, Katerina Koukiou wrote: > On Thu, Jul 12, 2018 at 05:19:27PM +0200, Andrea Bolognani wrote: > > +class Projects: > > + > > +def __init__(self): > > +try: > > +with open("./vars/mappings.yml", "r") as f: > > There is clear information where how to run the lcitool in the docs > in some patches befor so the relative paths that are used everywhere in > the code are not causing a problem. > Though IMO, I think it's clearer to have a variable (config > option, hardcoded, env variable or whatever you decide), storing the path of > these files so that this code is not dependent on relative paths. WDYT? Some of the paths, like vars/mappings.yml, are pretty much entirely arbitrary but others, like group_vars/all/main.yml, can't be changed because that's what Ansible expects. Additionally, none of the paths is repeated more than once in the script so using something like mappings_path = "./vars/mappings.yml" open(mappings_path, "r") instead of open("./vars/mappings.yml", "r") wouldn't IMHO buy us much. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 10/12] lcitool: Add projects information handling
On Tue, Jul 17, 2018 at 04:44:24PM +0200, Andrea Bolognani wrote: > On Tue, 2018-07-17 at 15:04 +0200, Katerina Koukiou wrote: > > On Thu, Jul 12, 2018 at 05:19:27PM +0200, Andrea Bolognani wrote: > > > +class Projects: > > > + > > > +def __init__(self): > > > +try: > > > +with open("./vars/mappings.yml", "r") as f: > > > > There is clear information where how to run the lcitool in the docs > > in some patches befor so the relative paths that are used everywhere in > > the code are not causing a problem. > > Though IMO, I think it's clearer to have a variable (config > > option, hardcoded, env variable or whatever you decide), storing the path of > > these files so that this code is not dependent on relative paths. WDYT? > > Some of the paths, like vars/mappings.yml, are pretty much entirely > arbitrary but others, like group_vars/all/main.yml, can't be changed > because that's what Ansible expects. > > Additionally, none of the paths is repeated more than once in the > script so using something like > > mappings_path = "./vars/mappings.yml" > open(mappings_path, "r") > > instead of > > open("./vars/mappings.yml", "r") > > wouldn't IMHO buy us much. I meant only having a variable for the playbook location. playbook_path = os.getenv('PLAYBOOK_PATH', './')) And then use os.path.join for all the others relative paths to the playbook path. So that you can actually run the lcitool script from wherever you want. Anyway, since this script usage is not wide, feel free to keep everything hardcoded. > > -- > Andrea Bolognani / Red Hat / Virtualization signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v2 10/12] lcitool: Add projects information handling
On Tue, 2018-07-17 at 19:55 +0200, Katerina Koukiou wrote: > I meant only having a variable for the playbook location. > > playbook_path = os.getenv('PLAYBOOK_PATH', './')) > > And then use os.path.join for all the others relative paths to the > playbook path. > So that you can actually run the lcitool script from wherever you want. That wouldn't be quite enough: you'd also have to make sure you call ansible / ansible-playbook like ANSIBLE_CONFIG="$PLAYBOOK_PATH/ansible.cfg" \ ansible --playbook-dir "$PLAYBOOK_PATH" ... otherwise it won't work. And at that point, the script might as well just figure out the base directory itself based on its own location :) Should be easy enough. I'll look into it. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list