[Fuego] [PATCH 2/2] ftc: use python-jenkins library to list jobs

Daniel Sangorrin daniel.sangorrin at toshiba.co.jp
Thu Mar 30 01:07:35 UTC 2017


> -----Original Message-----
> From: fuego-bounces at lists.linuxfoundation.org [mailto:fuego-bounces at lists.linuxfoundation.org] On Behalf Of Daniel Sangorrin
> > -----Original Message-----
> > From: Bird, Timothy [mailto:Tim.Bird at sony.com]
> > > -	job_list = get_jenkins_jobs(conf).keys()
> > > +	job_list = [job['name'] for job in server.get_jobs()]
> >
> > I'm not usually a fan of list comprehensions because often
> > they are difficult to read (IMHO).  But how can you pass up a chance
> > to replace a whole function with one line?!!  :-)
> 
> haha, they are powerful.
> 
> > python-jenkins rocks!
> >
> > Note that I plan to put server into conf.  So this might become:
> > job_list = job['name'] for job in conf.server.get_jobs()]
> 
> Ok, I got it.
> 
> > The reason for putting it inside conf is that the url for theserver
> > should be read from a configuration file, and might be overridable
> > for using ftc with a remote Jenkins instance.
> 
> Sounds good. I was also thinking about using ftc to control a remote
> server.
> 
> > Also, it might be nice to only instantiate 'server' on use.  I haven't
> > measured to see if there's much overhead to creating the server instance,
> > but if so it would be better to do delayed instantiation, as it's only needed
> > for commands that interact with Jenkins.
> 
> Yes, I also thought about it. I will fix it.

I checked this but there wasn't much overhead. You can stop jenkins and still
do 'ftc list-targets' because the instantiation doesn't mean that it actually
connects to the server.

Cheers
Daniel


> 
> > >  	job_list.sort()
> > >
> > >  	if not quiet: print "Jenkins jobs in this system:"
> > >  	for job in job_list:
> > > -		if not quiet: print "   ",
> > > -		print "%s" % (job)
> > > +		if not quiet:
> > > +			print "   %s" % (job)
> >
> > This is not right.  The quiet flag only controls the title and indent.
> > The change here makes the command completely silent.
> 
> Oh my bad, I thought it was some kind of errata. Maybe
> it's a good idea to indent print "   ", for readability.
> 
> > The purpose of 'quiet' in this case is to create
> > machine-readable output, so you could do something like:
> > ftc list-jobs -q | xargs <do-something-with-a-job-name>
> 
> I see, I wasn't aware of that.
> 
> Thanks for the review,
> Daniel
> 
> 
> _______________________________________________
> Fuego mailing list
> Fuego at lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego





More information about the Fuego mailing list