Hello gang,
take a look at the patch attached - both autotest_server and job.tag are stored in the autoqa_conf variable. To get the job.tag there, I had to do a little 'hack' - the autoqa_conf is stored in the control file as a string, so when creating this string, I add a parameter
jobtag = %s
to the [general] section of the config, and append
% (job.tag, )
behind the string, so it gets evaluated, when the control file is imported (i.e. when the job.tag value is known).
The respective part of the control file then looks like this:
autoqa_conf = ''' [test] smtpserver = localhost result_email = mail_from = autoqa@fedoraproject.org
[general] autotest_server = dhcp-30-103.brq.redhat.com jobtag = %s notification_email = local = false hookdir = /usr/share/autoqa testdir = /usr/share/autotest/client/site_tests
''' % (job.tag, )
so nothing needs to change in control files (no new parameter is required), only the tests will need minor adjustments in the initialize method:
def initialize(self, envr, name, kojitag, config): self.config = config_loader(config, self.tmpdir) autotest_server = self.config.get('general', 'autotest_server') jobtag = self.config.get('general', 'jobtag') self.autotest_url = "http://%s/results/%s/" % (autotest_server, jobtag)
And then of course the test needs to take advantage of the self.autotest_url variable (i.e. add the URL to the e-mail).
What do you think about this approach - I belive it's quite clean (apart from the minor job.tag 'hack') and hope you'll like it.
joza
-------
diff --git a/autoqa b/autoqa index efb94e3..cf8b6ee 100755 --- a/autoqa +++ b/autoqa @@ -27,6 +27,7 @@ import optparse import tempfile import StringIO import urlgrabber +import socket from ConfigParser import * from subprocess import call
@@ -37,6 +38,7 @@ conf = { 'testdir': '/usr/share/autotest/client/site_tests', 'hookdir': '/usr/share/autoqa', 'notification_email': '', + 'autotest_server': socket.gethostname(), } cfg_parser = SafeConfigParser() # used by prep_controlfile try: @@ -58,6 +60,10 @@ def prep_controlfile(controlfile, extradata): prepended with key='value' lines for each item in extradata and an 'autoqa_conf' variable which holds the contents of the autoqa configfile. ''' + # the jobtag must be evaluated on every single testrun (since it changes :-)) + # so line 'jobtag = %s' is added to the [general] part of the autoqa_conf + # string, and then is replaced with job.tag when the code is executed + cfg_parser.set('general', 'jobtag', '%s') controldata = open(controlfile) (fd, name) = tempfile.mkstemp(prefix='autoqa-control.') os.write(fd, '# -*- coding: utf-8 -*-\n\n') @@ -65,7 +71,7 @@ def prep_controlfile(controlfile, extradata): cfgdata = StringIO.StringIO() cfg_parser.write(cfgdata) cfgdata.seek(0) - os.write(fd, "autoqa_conf = '''\n%s'''\n\n" % cfgdata.read()) + os.write(fd, "autoqa_conf = '''\n%s''' %% (job.tag, )\n\n" % cfgdata.read()) except IOError: pass for k,v in extradata.iteritems(): @@ -139,6 +145,9 @@ parser.add_option('--local', action='store_true', dest='local', help='Do not schedule jobs - run test(s) directly on the local machine') parser.add_option('-l', '--list-tests', action='store_true', dest='listtests', help='list the tests for the given hookname - do not run any tests') +parser.add_option('--autotest-server', action='store', default=None, + help='Sets the autotest-server hostname. Used for creating URLs to results.\ +Hostname of the local machine is used by default.') # Read and validate the hookname # Check for no args, or just -h/--help if len(sys.argv) == 1 or sys.argv[1] in ('-h', '--help'): @@ -197,6 +206,11 @@ for arch in opts.arch: # N.B. process_testdata may grow new keyword arguments if we add new autoqa # args that add another loop here.. testdata = hook.process_testdata(opts, args, arch=arch) + if not 'autotest_server' in testdata.keys(): + if opts.autotest_server is not None: + testdata['autotest_server'] = opts.autotest_server + else: + testdata['autotest_server'] = conf['autotest_server'] # XXX FIXME: tests need to be able to indicate that they do not require # any specific arch (e.g. rpmlint can run on any arch) for test in testlist:
On Fri, 2010-04-23 at 11:30 +0200, Josef Skladanka wrote:
Hello gang,
take a look at the patch attached - both autotest_server and job.tag are stored in the autoqa_conf variable. To get the job.tag there, I had to do a little 'hack' - the autoqa_conf is stored in the control file as a string, so when creating this string, I add a parameter
jobtag = %s
to the [general] section of the config, and append
% (job.tag, )
behind the string, so it gets evaluated, when the control file is imported (i.e. when the job.tag value is known).
It took me a little while to understand the patch, but now that I get how it works.. that's really clever!
But there's two things:
1) I'd suggest creating a separate section for job-specific data, i.e.: if not cfg_parser.has_section('job'): cfg_parser.add_section('job') cfg_parser.set('job','tag','@JOBTAG@') # see below about @JOBTAG@
2) In my testing, cfg_parser.set('job','tag','%s') will raise: ValueError: invalid interpolation syntax in '%s' at position 0 (at least, it does on my F13 system). That's why I used '@JOBTAG@' above. But then we'd need to change the ' % (job.tag, )' part to: ... '''.replace('@JOBTAG@',job.tag) which is starting to get kind of messy.
We could do this a slightly simpler way. In autoqa:
os.write(fd, "autoqa_conf = '''\n%s\n[job]\ntag=%s''' %% (job.tag, )\n \n" % cfgdata.read())
This adds the job section and puts the magic bits into the control file so we get the job tag set. It's inelegant but I think it would work?
only the tests will need minor adjustments in the initialize method:
def initialize(self, envr, name, kojitag, config): self.config = config_loader(config, self.tmpdir) autotest_server = self.config.get('general', 'autotest_server') jobtag = self.config.get('general', 'jobtag') self.autotest_url = "http://%s/results/%s/" % (autotest_server,jobtag)
Probably we'd want to put this into the autoqa library, so it'd look more like: def initialize(self, envr, name, kojitag, config): self.config = config_loader(config, self.tmpdir) self.autotest_url = autoqa.util.make_autotest_url(config)
What do you think about this approach - I belive it's quite clean (apart from the minor job.tag 'hack') and hope you'll like it.
I like it a lot.
I'm still a little confused about why you can get away with doing cfg_parser.set('job','tag','%s') - maybe you're using F12 and SafeConfigParser behavior has changed?
Anyway, once we sort out how exactly to set things up so the magic interpolation happens in the control file as desired - and once we agree on what section/name to use for the job tag - I think we're really close to deploying this.
-w
Hello,
thank you for the feedback, Will, replies inside the text of your mail:
It took me a little while to understand the patch, but now that I get how it works.. that's really clever!
But there's two things:
- I'd suggest creating a separate section for job-specific data:
I agree, that makes sense.
- In my testing, cfg_parser.set('job','tag','%s') will raise:
ValueError: invalid interpolation syntax in '%s' at position 0 (at least, it does on my F13 system). That's why I used '@JOBTAG@' above. But then we'd need to change the ' % (job.tag, )' part to: ... '''.replace('@JOBTAG@',job.tag) which is starting to get kind of messy.
We could do this a slightly simpler way. In autoqa:
os.write(fd, "autoqa_conf = '''\n%s\n[job]\ntag=%s''' %% (job.tag, )\n \n" % cfgdata.read())
This adds the job section and puts the magic bits into the control file so we get the job tag set. It's inelegant but I think it would work?
Ad ValueError - This is probably caused by different versions of Python/SafeConfigParser - the interpolation happens on the server side which currently requires Python 2.4 (or at least packages of autotest I have from jlask do), so I'm using RHEL 5.4 as a server for my testing purposes. When I try the cfg_parser.set('job','tag','%s') on the RHEL machine, it works like charm, but newer Python fails on this line.
I'd suggest yet another way to do this
1) Using String Formatting Operations [1] to 'fool' ConfigParser, which works all through the Python versions (tested on FC12/13, RHEL5.4):
cfg_parser.set('job', 'tag', '%(jobtag)s') ... os.write(fd, "autoqa_conf = '''\n%s''' %% {'jobtag':job.tag}\n\n" % cfgdata.read())
2) Using Template Strings [2] (present from Python 2.4) which is a bit more greedy on the changes, but the possibility of future 'collision' with ConfigParser implementation should be virtually zero.
cfg_parser.set('job', 'tag', '${jobtag}') ... os.write(fd, "from string import Template\n\n") os.write(fd, "autoqa.conf = Template('''\n%s''').safe_substitute(jobtag=job.tag)\n\n" % cfgdata.read())
Which of the all suggested options do you guys like the most - every way has its positives and drawbacks, so I'd love to have your opinion on this.
Thanks
Joza
[1] http://docs.python.org/library/stdtypes.html#string-formatting-operations [2] http://www.python.org/dev/peps/pep-0292/
Probably we'd want to put this into the autoqa library, so it'd look more like: def initialize(self, envr, name, kojitag, config): self.config = config_loader(config, self.tmpdir) self.autotest_url = autoqa.util.make_autotest_url(config)
Agreed, I'll do that.
On Mon, 2010-05-03 at 13:28 +0200, Josef Skladanka wrote:
I'd suggest yet another way to do this
- Using String Formatting Operations [1] to 'fool' ConfigParser, which
works all through the Python versions (tested on FC12/13, RHEL5.4):
cfg_parser.set('job', 'tag', '%(jobtag)s') ... os.write(fd, "autoqa_conf = '''\n%s''' %% {'jobtag':job.tag}\n\n" %cfgdata.read())
I actually wrote up a version that did this, and discarded it as being perhaps more complicated than necessary. But since you came up with the same idea I'm guessing we both had the same intuition: someday we're probably going to end up passing more autotest variables into autoqa.conf, so having it be clear which format string corresponds to which variable is probably a good idea.
So: I like this method. But I might suggest rewriting the os.write call to be a little clearer, e.g.:
autoqa_conf_str = "autoqa_conf = '''\n%s'''" % cfgdata.read() # The key names in the following dict must correspond to the special # config values set above (search for e.g. 'jobtag' to see them). # The values in the dict get interpolated when autotest runs the # control file, thus putting those values into the config (and # passing them along to the tests) even though they don't exist yet. autoqa_conf_str += ' % {'jobtag':job.tag}\n\n' os.write(fd, autoqa_conf_str)
(Everyone is familiar with my deep love of really long comments, right?)
Does this seem reasonable?
-w
Hi,
On 05/05/2010 07:17 PM, Will Woods wrote:
On Mon, 2010-05-03 at 13:28 +0200, Josef Skladanka wrote: So: I like this method. But I might suggest rewriting the os.write call to be a little clearer, e.g.:
autoqa_conf_str = "autoqa_conf = '''\n%s'''" % cfgdata.read() # The key names in the following dict must correspond to the special # config values set above (search for e.g. 'jobtag' to see them). # The values in the dict get interpolated when autotest runs the # control file, thus putting those values into the config (and # passing them along to the tests) even though they don't exist yet. autoqa_conf_str += ' % {'jobtag':job.tag}\n\n' os.write(fd, autoqa_conf_str)(Everyone is familiar with my deep love of really long comments, right?)
Does this seem reasonable?
I love this, patch will follow.
But I have found out an issue (IDK if it's a real one) - the autoqa library for python does not get synced from the server to client (as tests do) - when i added the make_autotest_url() function to the autoqa.util library, i had to manually change the util.py file on the client. Is this correct behaviour or not? Because it means, that autotest clients can get easily out of sync from the server, every time the autoqa library is changed.
Joza
autoqa-devel@lists.fedorahosted.org
-
Josef Skladanka -
Will Woods