[PATCH] Added a support for sending comments into Bodhi, part 2

[Martin Krizek]

Hi all,

this is the patch from last week for sending comments to bodhi including some alterations
based on the suggestions that were mentioned here, a list follows:

* split _bodhi_already_commented into two functions
* new config parser that supports alternative config files
* a new section bodhi in autoqa.conf allowing turning the support on/off and setting time
span between posting the same comment/test result
* a few bug fixes

---

diff --git a/Makefile b/Makefile
index ac4ddeb..f3a14f9 100644
--- a/Makefile
+++ b/Makefile
@@ -22,6 +22,7 @@ install: build
 	install autoqa $(PREFIX)/usr/bin/
 	install -d $(PREFIX)/etc/autoqa
 	[ -f $(PREFIX)/etc/autoqa/autoqa.conf ] || install -m 0644 autoqa.conf
$(PREFIX)/etc/autoqa/
+	[ -f $(PREFIX)/etc/autoqa/fas.conf ] || install -m 0640 -g autotest fas.conf
$(PREFIX)/etc/autoqa
 	install -m 0644 repoinfo.conf $(PREFIX)/etc/autoqa/
 	install -d $(PREFIX)$(HOOK_DIR)
 	for h in hooks/*; do cp -a $$h $(PREFIX)$(HOOK_DIR); done
diff --git a/autoqa b/autoqa
index cb2d737..fd41503 100755
--- a/autoqa
+++ b/autoqa
@@ -1,7 +1,7 @@
 #!/usr/bin/python
 # autoqa-autotest: script to handle incoming requests from watchers and kick
 #                  off appropriate autotest jobs.
-# 
+#
 # Copyright 2009, Red Hat, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
@@ -30,8 +30,8 @@ import urlgrabber
 import socket
 import copy
 import fnmatch
-from ConfigParser import *
 from subprocess import call
+from autoqa.util import SingleConfigParser, getbool
 
 cfgfile = '/etc/autoqa/autoqa.conf'
 # Hardcoded defaults for the 'general' section
@@ -42,20 +42,11 @@ conf = {
     'notification_email': '',
     'autotest_server': socket.gethostname(),
 }
-cfg_parser = SafeConfigParser() # used by prep_controlfile
-try:
-    cfg_parser.read(cfgfile)
-    # override defaults with configfile values
-    for k,v in cfg_parser.items('general'):
-        conf[k] = v
-    # save final config state back to ConfigParser instance
-    for k,v in conf.items():
-        cfg_parser.set('general',k,v)
-except IOError: # no config file
-    pass
-except (NoSectionError, DuplicateSectionError, MissingSectionHeaderError), e:
-    print "ERROR: Could not parse %s: %s" % (cfgfile, str(e))
-    sys.exit(1)
+
+cfg_parser = SingleConfigParser()
+cfg_parser.read(cfgfile)
+conf = cfg_parser.get_section('general', conf) # used by prep_controlfile
+# we don't need to catch errors here, bcz we want autoqa to crash for invalid config
 
 def prep_controlfile(controlfile, extradata):
     '''Returns the name of a temporary file containing the given
controlfile,
@@ -97,7 +88,7 @@ def eval_test_vars(test, test_vars):
     Arguments:
         * test - name of the test for which control.autoqa should be executed
         * test_vars - dictionary with test variables that will be used for input
-    Returns: dictionary with test variables that have been evaluated (and 
+    Returns: dictionary with test variables that have been evaluated (and
     probably modified) by test's control.autoqa file
     '''
     cfile = open(os.path.join(conf['testdir'], test, 'control.autoqa'))
@@ -187,7 +178,7 @@ if not os.path.isdir(conf['hookdir']):
 known_hooks = [d for d in os.listdir(conf['hookdir']) if
os.path.isdir(os.path.join(conf['hookdir'], d))]
 
 # Set up the option parser
-parser = optparse.OptionParser(usage="%prog HOOKNAME [options] ...", 
+parser = optparse.OptionParser(usage="%prog HOOKNAME [options] ...",
     add_help_option=False)
 parser.add_option('-h', '--help', action='help',
     help='show this help message (or hook help message if HOOKNAME given) and \
@@ -232,7 +223,7 @@ hook.extend_parser(parser)
 args.pop(0) # dump hookname
 
 # Run the tests locally, or schedule them through autotest?
-run_local = (opts.local or (conf['local'].lower() == 'true'))
+run_local = (opts.local or getbool(conf['local']))
 
 if not opts.arch or run_local:
     opts.arch = ['noarch']
diff --git a/autoqa.conf b/autoqa.conf
index ed8662a..69278fe 100644
--- a/autoqa.conf
+++ b/autoqa.conf
@@ -23,3 +23,12 @@ result_email =
 mail_from = autoqa(a)fedoraproject.org
 # hostname or hostname:port of smtp server / mailhub to use for sending email
 smtpserver = localhost
+
+[bodhi]
+# If "true", test results (for tests utilizing this feature) will be sent
+# as comments to Fedora Update System (Bodhi). This requires that you have
+# Bodhi credentials filled in in fas.conf.
+send_bodhi_comments = false
+# how long (minutes) should we wait before posting the same comment to bodhi
+# by default 3 days (3*24*60 = 4320)
+bodhi_posting_comments_span = 4320
diff --git a/autoqa.spec b/autoqa.spec
index 261b20a..84b9efe 100644
--- a/autoqa.spec
+++ b/autoqa.spec
@@ -62,6 +62,7 @@ make build PYTHON=%{__python}
 rm -rf $RPM_BUILD_ROOT
 make install PREFIX=$RPM_BUILD_ROOT TEST_DIR=%{testdir} HOOK_DIR=%{hookdir}
PYTHON=%{__python}
 install -m 644 autoqa.conf repoinfo.conf $RPM_BUILD_ROOT%{_sysconfdir}/autoqa/
+install -m 640 -g autotest fas.conf $RPM_BUILD_ROOT%{_sysconfdir}/autoqa/
 # front-ends/israwhidebroken
 mv %{buildroot}%{_bindir}/start-israwhidebroken %{buildroot}%{_sbindir}/
 mv %{buildroot}%{_bindir}/israwhidebroken.wsgi %{buildroot}%{_sbindir}/
@@ -78,6 +79,7 @@ rm -rf $RPM_BUILD_ROOT
 %doc README LICENSE TODO autoqa.cron
 %dir %{_sysconfdir}/autoqa
 %config(noreplace) %{_sysconfdir}/autoqa/autoqa.conf
+%config(noreplace) %{_sysconfdir}/autoqa/fas.conf
 %config %{_sysconfdir}/autoqa/repoinfo.conf
 %config(noreplace) %{testdir}/rats_sanity/irb.cfg
 %dir %attr(0775,root,autotest) %{_localstatedir}/cache/autoqa
diff --git a/doc/test_class.py.template b/doc/test_class.py.template
index 590bd16..c19074a 100644
--- a/doc/test_class.py.template
+++ b/doc/test_class.py.template
@@ -32,7 +32,7 @@ from autotest_lib.client.bin import utils
 
 # Your class name must match file name (without .py) and also run_test line in
 # its control file.
-class testclassname(AutoQATest): # <-- UPDATE Classname
+class testclassname(AutoQATest): # <-- UPDATE class name
     version = 1 # increment this if setup() changes
 
     # All methods below may receive arbitrary number of arguments that you
@@ -53,7 +53,7 @@ class testclassname(AutoQATest): # <-- UPDATE Classname
     # method - if you don't need to initialize anything, delete this block.
     #@ExceptionCatcher()
     #def initialize(self, config, **kwargs): #**kwargs needs to stay
-    #    super(testclassname, self).initialize(config) # <-- UPDATE Classname
+    #    super(testclassname, self).initialize(config) # <-- UPDATE class name
     #    #your extra initialization code goes here
 
     # This is where the test code actually gets run. It's the only required
@@ -65,6 +65,7 @@ class testclassname(AutoQATest): # <-- UPDATE Classname
     #   self.highlights: important lines to notice (string or list of strings)
     @ExceptionCatcher()
     def run_once(self, some_params, **kwargs): #**kwargs needs to stay
+        super(testclassname, self).run_once() # <-- UPDATE class name
         cmd = 'test_binary --param %s' % some_params
         self.outputs = utils.system_output(cmd, retain_output=True)
 
diff --git a/fas.conf b/fas.conf
new file mode 100644
index 0000000..5fc322e
--- /dev/null
+++ b/fas.conf
@@ -0,0 +1,6 @@
+# FAS (Fedora Accounts System) credentials
+# These credentials are used when reporting results in the name of AutoQA,
+# i.e. posting a comment into Bodhi
+[fas]
+username =
+password =
diff --git a/hooks/post-tree-compose/watch-composes.py
b/hooks/post-tree-compose/watch-composes.py
index 64a7fd3..a9d4e23 100755
--- a/hooks/post-tree-compose/watch-composes.py
+++ b/hooks/post-tree-compose/watch-composes.py
@@ -28,6 +28,7 @@ import sys
 import subprocess
 from autoqa.repoinfo import repoinfo
 import optparse
+from autoqa.util import getbool
 
 # Set up the option parser
 parser = optparse.OptionParser(description='A utility to watch a set of \
@@ -40,7 +41,7 @@ parser.add_option('--dryrun', '--dry-run',
action='store_true',
 repoinfo.setarch('%%a') # two %% because of ConfigParser interpolation
 
 # Get the list of repos to watch from repoinfo (see /etc/autoqa/repoinfo.conf)
-watchcomposes = [r for r in repoinfo.repos() if repoinfo.get(r,'composes') ==
'yes']
+watchcomposes = [r for r in repoinfo.repos() if
getbool(repoinfo.get(r,'composes'))]
 
 # Setup a cache path
 cachedir = '/var/cache/autoqa/watch-composes'
diff --git a/lib/python/bodhi_utils.py b/lib/python/bodhi_utils.py
index 9c0adbc..94e1a7d 100644
--- a/lib/python/bodhi_utils.py
+++ b/lib/python/bodhi_utils.py
@@ -18,9 +18,15 @@
 #
 # Authors:
 #     Will Woods <wwoods(a)redhat.com&gt;
+#     Martin Krizek <mkrizek(a)redhat.com&gt;
 
 import fedora.client
 import time
+import sys
+import re
+from datetime import datetime
+from util import SingleConfigParser, getbool
+import ConfigParser
 
 def bodhitime(timestamp):
     '''Convert timestamp (seconds since Epoch, assumed to be local time) to
a
@@ -46,3 +52,199 @@ def bodhi_list(params, limit=100):
         updates += r['updates']
         params['tg_paginate_no'] += 1
     return updates
+
+def _bodhi_already_commented(update, user, testname, arch):
+    '''Check if the comment is already posted.
+
+       Args:
+       update -- The *title* of the update
+       user -- username that posted the comment
+       testname -- the name of the test
+       arch -- tested architecture
+
+       Returns:
+       Tuple containing old result and time when the last comment was posted,
+       if none comment is posted already, tuple will be empty.
+    '''
+    params = dict()
+    params['package'] = update
+    res = bodhi_list(params)
+    return _check_already_commented(res[0], user, testname, arch)
+
+def _check_already_commented(update, user, testname, arch):
+    '''Check if the comment is already posted.
+
+       Args:
+       update -- The *title* of the update
+       user -- username that posted the comment
+       testname -- the name of the test
+       arch -- tested architecture
+
+       Returns:
+       Tuple containing old result and time when the last comment was posted,
+       if none comment is posted already, tuple will be empty.
+    '''
+    comment_re = r'AutoQA:[\s]+%s[\s]+test[\s]+(\w+)[\s]+on[\s]+%s' % (testname,
arch)
+    old_result = ''
+    comment_time = ''
+
+    for comment in update['comments']:
+        if comment['author'] == user:
+            m = re.match(comment_re, comment['text'])
+            if m == None:
+                continue
+            old_result = m.group(1)
+            comment_time = comment['timestamp']
+
+    return (old_result, comment_time)
+
+def _is_bodhi_testresult_needed(old_result, comment_time, result, time_span):
+    '''Check if the comment is meant to be posted.
+
+       Args:
+       old_result -- the result of the last test
+       comment_time -- the comment time of the last test
+       result -- the result of the test
+       time_span -- waiting period before posting the same comment
+
+       Returns:
+       True if the comment will be posted, False otherwise.
+    '''
+    # the first comment or a comment with different result, post it
+    if not old_result or old_result != result:
+        return True
+
+    # If we got here, it means that the comment with the same result has been
+    # already posted, we now need to determine whether we can post the
+    # comment again or not.
+    # If the previous result is *not* 'FAILED', we won't post it in order not
to
+    # spam developers.
+    # If the previous result *is* 'FAILED', we will need to check whether given
+    # time span expired, if so, we will post the same comment again to remind
+    # a developer about the issue.
+
+    if result != 'FAILED':
+        return False
+
+    posted_datetime = datetime.strptime(comment_time, '%Y-%m-%d %H:%M:%S')
+    delta = (datetime.utcnow() - posted_datetime)
+    # total_seconds() is introduced in python 2.7, until 2.7 is everywhere...
+    total_seconds = (delta.microseconds + (delta.seconds + delta.days * 24 * 3600) *
10**6) / 10**6
+    minutes = total_seconds/60.0
+    if minutes < time_span:
+        return False
+
+    return True
+
+def bodhi_post_testresult(update, testname, result, url, config, arch = 'noarch',
karma = 0):
+    '''Post comment and karma to bodhi
+
+       Args:
+       update -- the *title* of the update comment on
+       testname -- the name of the test
+       result -- the result of the test
+       url -- url of the result of the test
+       config -- autoqa config
+       arch -- tested architecture (default 'noarch')
+       karma -- karma points (default 0)
+
+       Returns:
+       True if comment was posted successfully or comment wasn't meant to be
+       posted (either posting is turned off or comment was already posted),
+       False otherwise.
+    '''
+    err_msg = 'Could not post a comment to bodhi'
+    conffiles = ['fas.conf', '/etc/autoqa/fas.conf']
+    try:
+        cfg_parser = SingleConfigParser()
+        cfg_parser.read_single(conffiles)
+        fas = cfg_parser.get_section('fas')
+    except (IOError, ConfigParser.Error), e:
+        print >> sys.stderr, "ERROR: Couldn't read none of config files:
%s" % conffiles
+        print >> sys.stderr, e
+        return False
+
+    if not update or not testname or not result or url == None:
+        sys.stderr.write('Incomplete arguments!\n%s\n' % err_msg)
+        return False
+
+    try:
+        if not getbool(config.get('bodhi', 'send_bodhi_comments')):
+            print 'Sending bodhi comments is turned off. Test result will NOT be
sent.'
+            return True
+    except KeyError:
+        print 'Sending bodhi comments is turned off. Test result will NOT be
sent.'
+        # option missing -> it's false, do not send it (but return True since
+        # it's intentional, not an error)
+        return True
+
+    try:
+        user = fas['username']
+        pswd = fas['password']
+        if not user or not pswd:
+            raise KeyError
+    except KeyError:
+        sys.stderr.write('Conf file containing FAS credentials is
incomplete!\n%s\n' % err_msg)
+        return False
+
+    comment = 'AutoQA: %s test %s on %s. The result can be found at: %s.' \
+               % (testname, result, arch, url)
+    try:
+        (old_result, comment_time) = _bodhi_already_commented(update, user, testname,
arch)
+        time_span = int(config.get('bodhi',
'bodhi_posting_comments_span'))
+
+        if not _is_bodhi_testresult_needed(old_result, comment_time, result, time_span):
+            print 'The test result already posted to bodhi.'
+            return True
+
+        bodhi = fedora.client.BodhiClient(username=user, password=pswd)
+
+        if not bodhi.comment(update, comment, karma):
+            sys.stderr.write('%s\n') % err_msg
+            return False
+
+        print 'The test result was sent to bodhi successfully.'
+    except Exception, e:
+        sys.stderr.write('An error occured: %s\n' % e)
+        sys.stderr.write('Could not connect to bodhi!\n%s\n' % err_msg)
+        return False
+
+    return True
+
+def _self_test():
+    '''
+    Simple self test.
+    '''
+    from datetime import timedelta
+    from ConfigParser import SafeConfigParser
+    cfg_parser = SafeConfigParser()
+    cfg_parser.read('/etc/autoqa/autoqa.conf')
+    time_span = int(cfg_parser.get('bodhi',
'bodhi_posting_comments_span'))
+    try:
+        print '1. Test:',
+        assert _is_bodhi_testresult_needed('PASSED', datetime.now,
'PASSED', time_span) == False
+        print 'Passed'
+        print '2. Test:',
+        assert _is_bodhi_testresult_needed('FAILED', datetime.now,
'PASSED', time_span) == True
+        print 'Passed'
+        print '3. Test:',
+        assert _is_bodhi_testresult_needed('PASSED', datetime.now,
'FAILED', time_span) == True
+        print 'Passed'
+        print '4. Test:',
+        date = (datetime.utcnow() - timedelta(minutes=time_span)).\
+        strftime('%Y-%m-%d %H:%M:%S')
+        assert _is_bodhi_testresult_needed('FAILED', date, 'FAILED',
time_span) == True
+        print 'Passed'
+        print '5. Test:',
+        date = datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S')
+        assert _is_bodhi_testresult_needed('FAILED', date, 'FAILED',
time_span) == False
+        print 'Passed'
+        print '6. Test:',
+        assert _is_bodhi_testresult_needed('', '', 'FAILED',
time_span) == True
+        print 'Passed'
+    except AssertionError:
+        print 'Failed [!!!]'
+
+if __name__ == '__main__':
+    _self_test()
+
diff --git a/lib/python/test.py b/lib/python/test.py
index d59d765..29bb96c 100644
--- a/lib/python/test.py
+++ b/lib/python/test.py
@@ -46,7 +46,7 @@ class AutoQATest(test.test, object):
 
     @ExceptionCatcher()
     def run_once(self, **kwargs):
-        pass
+        os.chdir(self.bindir) # easiest way for tests to find their test scripts, config
files, etc
 
     def process_exception(self, exc):
         self._convert_list_variables()
diff --git a/lib/python/util.py b/lib/python/util.py
index b6d0013..b34259c 100644
--- a/lib/python/util.py
+++ b/lib/python/util.py
@@ -18,6 +18,7 @@
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 #
 # Author: Will Woods <wwoods(a)redhat.com&gt;
+#         Martin Krizek <mkrizek(a)redhat.com&gt;
 
 import os
 import sys
@@ -29,6 +30,7 @@ import urlgrabber.progress
 import config
 import urllib
 import socket
+import ConfigParser
 
 def timestamp_to_compose_id(timestamp=None, serial=1):
     if not timestamp:
@@ -180,4 +182,56 @@ def make_autotest_url(config):
     autotest_url = "http://%s/results/%s/" % (autotest_server, jobtag)
     return(autotest_url)
 
+class SingleConfigParser(ConfigParser.SafeConfigParser):
+    '''This is a subclassed SafeConfigParser. It has several additional
methods
+       serving our needs.
+    '''
+    def read_single(self, filenames):
+        '''This is the same as the read() method, but it parsers only the
first
+           available config file from the list, not all of them.
+        '''
+        if isinstance(filenames, str):
+            filenames = [filenames]
+        for f in filenames:
+            retval = self.read(f)
+            if retval:
+                # file read, don't read any other
+                return retval
+        else:
+            return []
+
+    def get_section(self, section, default_conf={}):
+        '''Read the whole section and convert it into a dictionary (with
string
+           values).
+
+           Args:
+           section -- section of the config to be retrieved
+           default_conf -- default configuration values
+
+           Returns:
+           Dictionary containing retrieved data on success.
+
+           Throws:
+           ConfigParser.NoSectionError -- when no such section exists
+        '''
+        items = dict(self.items(section))
+        retval = default_conf.copy()
+        retval.update(items)
+        return retval
+
+def getbool(string):
+    '''Converts a string into a boolean.
+       Recognizes many usual strings: 1/0, yes/no, true/false, on/off; also
+       empty strings.
+       Throws ValueError if the string can't be recognized.
+    '''
+    boolean_states = {'1': True, 'yes': True,  'true': True,  
'on': True,
+                      '0': False, 'no': False, 'false': False,
'off': False}
+    if not string:
+        return False
+
+    string = string.lower()
+    if string not in boolean_states:
+        raise ValueError, 'Not a boolean: %s' % string
 
+    return boolean_states[string]
diff --git a/tests/conflicts/conflicts.py b/tests/conflicts/conflicts.py
index 5a93a2b..c9a4667 100644
--- a/tests/conflicts/conflicts.py
+++ b/tests/conflicts/conflicts.py
@@ -34,11 +34,11 @@ class conflicts(AutoQATest):
 
     @ExceptionCatcher()
     def run_once(self, baseurl, parents, name, **kwargs):
+        super(conflicts, self).run_once()
         if name:
             name = "%s-%s" % (name, autoqa.util.get_basearch())
         else:
             name = baseurl
-        os.chdir(self.bindir)
         cmd = './potential_conflict.py --tempcache --newest'
         cmd += ' --repofrompath=target,%s --repoid=target' % baseurl
         count = 1
diff --git a/tests/helloworld/helloworld.py b/tests/helloworld/helloworld.py
index a0bd1d4..1a96ef5 100644
--- a/tests/helloworld/helloworld.py
+++ b/tests/helloworld/helloworld.py
@@ -25,6 +25,7 @@ class helloworld(AutoQATest):
 
     @ExceptionCatcher()
     def run_once(self, *args, **kwargs):
+        super(helloworld, self).run_once()
         self.summary = 'Hello, World!'
         self.outputs = "===Printing passed params===\n"
         for arg in args:
diff --git a/tests/initscripts/initscripts.py b/tests/initscripts/initscripts.py
index e0f95bb..8e35e35 100644
--- a/tests/initscripts/initscripts.py
+++ b/tests/initscripts/initscripts.py
@@ -101,6 +101,7 @@ class initscripts(AutoQATest):
 
     @ExceptionCatcher()
     def run_once(self, kojitag, **kwargs):
+        super(initscripts, self).run_once()
         if kwargs['hook'] == 'post-koji-build':
             envrs = [kwargs['envr']]
             update_id = kwargs['envr']
diff --git a/tests/rats_install/rats_install.py b/tests/rats_install/rats_install.py
index 552381a..23c2015 100644
--- a/tests/rats_install/rats_install.py
+++ b/tests/rats_install/rats_install.py
@@ -52,11 +52,11 @@ class rats_install(AutoQATest):
 
     @ExceptionCatcher()
     def run_once(self, baseurl, name, image_url="", boot_args="",
**kwargs):
+        super(rats_install, self).run_once()
         if name:
             name = "%s-%s" % (name, util.get_basearch())
         else:
             name = baseurl
-        os.chdir(self.bindir)
         cmd = "./install.py -s %s -l %s" % (self.tmpdir, self.resultsdir)
         if image_url != "":
             cmd += " -i %s" % image_url
diff --git a/tests/rats_sanity/rats_sanity.py b/tests/rats_sanity/rats_sanity.py
index d6a6357..19116ef 100644
--- a/tests/rats_sanity/rats_sanity.py
+++ b/tests/rats_sanity/rats_sanity.py
@@ -40,11 +40,11 @@ class rats_sanity(AutoQATest):
 
     @ExceptionCatcher()
     def run_once(self, baseurl, parents, name, **kwargs):
+        super(rats_install, self).run_once()
         if name:
             name = "%s-%s" % (name, util.get_basearch())
         else:
             name = baseurl
-        os.chdir(self.bindir)
         cmd = "./sanity.py -s %s -l %s" % (self.tmpdir, self.resultsdir)
         cmd += " %s" % baseurl
         self.result = None
diff --git a/tests/repoclosure/repoclosure.py b/tests/repoclosure/repoclosure.py
index b27b684..f167722 100644
--- a/tests/repoclosure/repoclosure.py
+++ b/tests/repoclosure/repoclosure.py
@@ -31,6 +31,7 @@ class repoclosure(AutoQATest):
 
     @ExceptionCatcher()
     def run_once(self, baseurl, parents='', name='', **kwargs):
+        super(repoclosure, self).run_once()
         if name:
             name = "%s-%s" % (name, autoqa.util.get_basearch())
         else:
diff --git a/tests/rpmguard/rpmguard.py b/tests/rpmguard/rpmguard.py
index 588a58f..bf973f0 100644
--- a/tests/rpmguard/rpmguard.py
+++ b/tests/rpmguard/rpmguard.py
@@ -43,6 +43,7 @@ class rpmguard(AutoQATest):
 
     @ExceptionCatcher()
     def run_once(self, kojitag, **kwargs):
+        super(rpmguard, self).run_once()
         if kwargs['hook'] == 'post-koji-build':
             envrs = [kwargs['envr']]
             update_id = kwargs['envr']
diff --git a/tests/rpmlint/rpmlint.py b/tests/rpmlint/rpmlint.py
index 5944c78..1c06a8b 100644
--- a/tests/rpmlint/rpmlint.py
+++ b/tests/rpmlint/rpmlint.py
@@ -43,6 +43,7 @@ class rpmlint(AutoQATest):
 
     @ExceptionCatcher()
     def run_once(self, kojitag, **kwargs):
+        super(rpmlint, self).run_once()
         if kwargs['hook'] == 'post-koji-build':
             envrs = [kwargs['envr']]
             update_id = kwargs['envr']
diff --git a/tests/upgradepath/upgradepath.py b/tests/upgradepath/upgradepath.py
index c53285f..a99ac2f 100755
--- a/tests/upgradepath/upgradepath.py
+++ b/tests/upgradepath/upgradepath.py
@@ -86,6 +86,7 @@ class upgradepath(AutoQATest):
 
     @ExceptionCatcher()
     def run_once(self, envrs, kojitag, **kwargs):
+        super(upgradepath, self).run_once()
         update_id = kwargs['name'] or kwargs['id']
 
         # Get a list of all repos we monitor (currently not -testing)


---

Martin

[Kamil Paral]

----- "Martin Krizek" <mkrizek(a)redhat.com&gt; wrote:

>  Hi all,
>  
>  this is the patch from last week for sending comments to bodhi
>  including some alterations based on the suggestions that were
>  mentioned here, a list follows:
>  
>  * split _bodhi_already_commented into two functions
>  * new config parser that supports alternative config files
>  * a new section bodhi in autoqa.conf allowing turning the support
>  on/off and setting time span between posting the same comment/test
>  result
>  * a few bug fixes 
As usual, you can also view this diff as:

$ git diff origin/master origin/mkrizek

And as usual, I closely worked with mkrizek on this patch, so it has already
ACK from me :)

Kamil

[James Laska]

On Fri, 2010-12-10 at 05:10 -0500, Martin Krizek wrote:
>  +class SingleConfigParser(ConfigParser.SafeConfigParser):
>  +    '''This is a subclassed SafeConfigParser. It has several
>  additional methods
>  +       serving our needs.
>  +    '''
>  +    def read_single(self, filenames):
>  +        '''This is the same as the read() method, but it parsers only
>  the first
>  +           available config file from the list, not all of them.
>  +        '''
>  +        if isinstance(filenames, str):
>  +            filenames = [filenames]
>  +        for f in filenames:
>  +            retval = self.read(f)
>  +            if retval:
>  +                # file read, don't read any other
>  +                return retval
>  +        else:
>  +            return []
>  +
>  +    def get_section(self, section, default_conf={}):
>  +        '''Read the whole section and convert it into a dictionary
>  (with string
>  +           values).
>  +
>  +           Args:
>  +           section -- section of the config to be retrieved
>  +           default_conf -- default configuration values
>  +
>  +           Returns:
>  +           Dictionary containing retrieved data on success.
>  +
>  +           Throws:
>  +           ConfigParser.NoSectionError -- when no such section exists
>  +        '''
>  +        items = dict(self.items(section))
>  +        retval = default_conf.copy()
>  +        retval.update(items)
>  +        return retval
>  +
>  +def getbool(string):
>  +    '''Converts a string into a boolean.
>  +       Recognizes many usual strings: 1/0, yes/no, true/false,
>  on/off; also
>  +       empty strings.
>  +       Throws ValueError if the string can't be recognized.
>  +    '''
>  +    boolean_states = {'1': True, 'yes': True,  'true': True,  
> 'on':
>  True,
>  +                      '0': False, 'no': False, 'false': False,
> 'off':
>  False}
>  +    if not string:
>  +        return False
>  +
>  +    string = string.lower()
>  +    if string not in boolean_states:
>  +        raise ValueError, 'Not a boolean: %s' % string
>   
>  +    return boolean_states[string]  
Everything looks good.  Only one comment regarding the getbool() method.
Doesn't SafeConfigParser provide different methods to return a value of
a specific type?  For example, getboolean(), getfloat(), getint().
Should we be using (or overloading in the case of getboolean) those
built-in methods instead?

>  http://docs.python.org/library/configparser.html 
Thanks,
James