On Fri, 2010-12-03 at 03:44 -0500, Martin Krizek wrote:
Hi all,
this patch (
https://fedorahosted.org/autoqa/ticket/205) allows AutoQA
to send a test result as a comment to bodhi.
Yay! Awesome work, Martin. A couple comments below:
+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.
+ '''
+ bodhi = fedora.client.BodhiClient()
+ res = bodhi.query(package=update)
+ comment_re = r'AutoQA:[\s]+%s[\s]+test[\s]+(\w+)[\s]+on[\s]+%s' % (testname,
arch)
+ old_result = ''
+ comment_time = ''
+
+ for update in res['updates']:
+ 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)
It would be really helpful if you split out the comment-parsing into its
own function, something like this:
def _bodhi_already_commented(update, user, testname, arch):
'''[docstring goes here]'''
bodhi = fedora.client.BodhiClient()
res = bodhi.query(package=update)
return _check_already_commented(res['updates'][0], user, testname, arch)
def _already_commented(update, user, testname, arch):
'''[docstring goes here]'''
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)
This allows the use of _already_commented in other places - e.g. to scan
a lot of updates with only one network access:
params = {'release': 'F14', 'status': 'testing',
'request': 'stable'}
for update in bodhi_list(params):
if _already_commented(update):
...
This happens to be something we're going to need to use in depcheck, to
figure out which of the unpushed pending packages have already passed a
previous depcheck.
In general, I think, it's best to try to keep data parsing separate from
network fetching. It lets the user fetch as much data as they want (or
fetch things by other methods) and still use the same parsing code.
-w