Re: [PATCH] Added a support for sending comments into Bodhi

[Kamil Paral]

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

>  Hi all,
>  
>  this patch (https://fedorahosted.org/autoqa/ticket/205) allows AutoQA
>  to send a test result as a comment to bodhi. You can turn on/off the
>  support by setting 'send_bodhi_comments' in /etc/autoqa/autoqa.conf to
>  true/false. Once the test is completed it will call the
>  bodhi_post_testresult function from lib/python/bodhi_utils.py. For
>  instance, the usage in upgradepath test would be:
>  
>  > bodhi_post_testresult(kwargs['name'], self.__class__.__name__,
>  self.result, self.autotest_url, self.config),
>  
>  note that kwargs['name'] must contain title of the update, for now.
>  Thanks to Luke Macken, there will be a support for posting comments by
>  UPDATEID in the next release of bodhi.
>  
>  The comment will be posted only if the support is turned on and if the
>  same comment is NOT already posted. The only exception are FAILED
>  results. Those will be sent again to remind the developer about the
>  issue. However, we do not want to send them FAILED results every time
>  the test failed, obviously. So there is a variable
>  BODHI_POSTING_COMMENT_SPAN in lib/python/bodhi_utils.py which tells
>  the script how long it should wait before posting the same comment
>  again. Note that the test result could be of the same result (FAILED),
>  but it could fail from a different reason than it did last time.
>  That's why the comment contains url to the test result, so developers
>  can check it. The format of the comments is as follows:
>  
>  > AutoQA: *test_name* test *result* on *arch*. The result can be found
>  at: *url*
>  
>  for example,
>  > AutoQA: upgradepath test PASSED on noarch. The result can be found
>  at: http://server.com/results/14-root/client.com/
>  
>  FAS (Fedora Accounts System) credentials for logging into bodhi and
>  sending comments are stored in /etc/autoqa/fas.conf.
>  
>  If you have any questions, please do ask. 
Thanks, Martin, for this patch. Just a few more remarks:

1. You can find the same code in the mkrizek branch.
2. This patch adds *support* for sending bodhi comments, but it does
   not enable it yet for any of our tests.
3. BODHI_POSTING_COMMENT_SPAN (comment duplication protection) is 
   currently set to 3 days
4. I have reviewed this patch with Martin throughout its developments,
   it has ACK from me. But please point out deficiencies if you see 
   some.
5. We will need to test this feature more before final release. Martin
   did a few manual tests, but that's not enough. My best idea is that
   we could use our development server, deploy the code there, deactivate
   actual bodhi sending code and just intercept the calls and log them.
   We would have it running for a week and only then we can be quite
   sure it will really work once we make a new release and use it for
   production machine.

[PATCH] Added a support for sending comments into Bodhi

[James Laska]

Merging my response for both mails into one.  I hope that's not too
confusing.

On Fri, 2010-12-03 at 12:45 -0500, Kamil Paral wrote:
>  ----- "Martin Krizek" <mkrizek(a)redhat.com&gt; wrote:
>  
>  > Hi all,
>  > 
>  > this patch (https://fedorahosted.org/autoqa/ticket/205) allows AutoQA
>  > to send a test result as a comment to bodhi. You can turn on/off the
>  > support by setting 'send_bodhi_comments' in /etc/autoqa/autoqa.conf to
>  > true/false. Once the test is completed it will call the
>  > bodhi_post_testresult function from lib/python/bodhi_utils.py. For
>  > instance, the usage in upgradepath test would be:
>  > 
>  > > bodhi_post_testresult(kwargs['name'], self.__class__.__name__,
>  > self.result, self.autotest_url, self.config),
>  > 
>  > note that kwargs['name'] must contain title of the update, for now.
>  > Thanks to Luke Macken, there will be a support for posting comments by
>  > UPDATEID in the next release of bodhi.
>  > 
>  > The comment will be posted only if the support is turned on and if the
>  > same comment is NOT already posted. The only exception are FAILED
>  > results. Those will be sent again to remind the developer about the
>  > issue. However, we do not want to send them FAILED results every time
>  > the test failed, obviously. So there is a variable
>  > BODHI_POSTING_COMMENT_SPAN in lib/python/bodhi_utils.py which tells
>  > the script how long it should wait before posting the same comment
>  > again. Note that the test result could be of the same result (FAILED),
>  > but it could fail from a different reason than it did last time.
>  > That's why the comment contains url to the test result, so developers
>  > can check it. The format of the comments is as follows:
>  > 
>  > > AutoQA: *test_name* test *result* on *arch*. The result can be found
>  > at: *url*
>  > 
>  > for example,
>  > > AutoQA: upgradepath test PASSED on noarch. The result can be found
>  > at: http://server.com/results/14-root/client.com/
>  > 
>  > FAS (Fedora Accounts System) credentials for logging into bodhi and
>  > sending comments are stored in /etc/autoqa/fas.conf.
>  > 
>  > If you have any questions, please do ask.
>  
>  Thanks, Martin, for this patch. Just a few more remarks:
>  
>  1. You can find the same code in the mkrizek branch.
>  2. This patch adds *support* for sending bodhi comments, but it does
>     not enable it yet for any of our tests.
>  3. BODHI_POSTING_COMMENT_SPAN (comment duplication protection) is 
>     currently set to 3 days 
Does it make sense to store/access this value in autoqa.conf in a
[bodhi] section?  Also, now that I think of it (see below), shouldn't
the bodhi server URL be a configuration option.  Unrelated to this
patchset, but this also applies to the koji server URL?

>  4. I have reviewed this patch with Martin throughout its
> developments,
>     it has ACK from me. But please point out deficiencies if you see 
>     some. 
Everything looks really good.  Nice work Martin and Kamil!  I've added a
few minor thoughts below.

>  +def get_cfg(cfgfile, section, default_conf = {}):
>  +    '''Get data from config file
>  +    
>  +       Args:
>  +       cfgfile -- config file name
>  +       section -- section of the config to be retrieved
>  +       default_conf -- default configuration values
>  +
>  +       Returns:
>  +       Dictionary containing retrieved data on success.
>  +    ''' 
Can we make get_cfg() accept a list of cfg files to try, not just a
single file (see similar example in lib/python/repoinfo.py)?  I've been
meaning to do this for the 'autoqa' script next time I'm in there.
Mainly because it makes running tests/watchers directly from a git
check-out difficult if the code only looks in '/etc/autoqa/autoqa.conf'.
With repoinfo, it looks for both
['repoinfo.conf,'/etc/autoqa/repoinfo.conf'].  Speaking of, should the
'autoqa' script to use the new get_cfg() method to access it's conf?

>  +        if not _is_bodhi_testresult_needed(old_result, comment_time,
> result):
>  +            print 'The test result already posted to bodhi.'
>  +            return True 
Does that 'print' statement show up in our autoqa logs so we know when
it decided not to post feedback into bodhi?

>  +def _self_test():
>  +    '''
>  +    Simple self test.
>  +    ''' 
Kudos!! :)

>  def run_once(self, **kwargs):
>  -        pass
>  +        os.chdir(self.bindir) # easiest way for tests to find their test scripts, config
> files, etc
>    
It looks like there are some changes to lib/python/test.py
(os.chdir(self.bindir)) and the test templates.  Are those changes
related to the bodhi comment support?  Does changing the PWD for all our
run_once() tests alter there outcome?

>  @ExceptionCatcher()
>       def run_once(self, baseurl, parents, name, **kwargs):
>  +        super(conflicts, self).run_once()
>           if name: 
Same as above, is this related/required by the bodhi comment support?
Also, if this is needed, we'll want to update the documentation?

>  5. We will need to test this feature more before final release.
> Martin
>     did a few manual tests, but that's not enough. My best idea is that
>     we could use our development server, deploy the code there, deactivate
>     actual bodhi sending code and just intercept the calls and log them.
>     We would have it running for a week and only then we can be quite
>     sure it will really work once we make a new release and use it for
>     production machine. 
lmacken informs me that we can use a staging bodhi instance for testing
comments (https://admin.stg.fedoraproject.org/updates).  Email
notifications are disabled, so we shouldn't have angry maintainers
hunting us down as we test this new support :)

>  12:57:18   lmacken: jlaska: ok, I just disabled email notifications
> in
>  our staging instance 
Thanks,
James