Differential D542

bodhi_comment_directive: only post comments on FAILED or change state
ClosedPublic
Actions

Authored by kparal on Aug 28 2015, 10:58 AM.

Details

Reviewers
tflink
jskladan
Maniphest Tasks
T583: Fix posting Bodhi comments
Commits
rLTRNf754991c3dc9: bodhi_comment_directive: only post comments on FAILED or change state
Summary

This will decrease the number of emails we send to developers.

Test Plan

battle tested with bodhi.stg

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kparal retitled this revision from to bodhi_comment_directive: only post comments on FAILED or change state.Aug 28 2015, 10:58 AM
kparal updated this object.
kparal edited the test plan for this revision. (Show Details)
kparal added reviewers: tflink, jskladan.
kparal added a task: T583: Fix posting Bodhi comments.
kparal added a comment.Aug 28 2015, 11:01 AM

@tflink, if there are no review concerns and I'm already offline, please feel free to push this yourself and deploy it (and enable it:)). Thanks!

tflink accepted this revision.Aug 28 2015, 3:16 PM
This revision is now accepted and ready to land.Aug 28 2015, 3:16 PM
Closed by commit rLTRNf754991c3dc9: bodhi_comment_directive: only post comments on FAILED or change state (authored by kparal). · Explain WhyAug 28 2015, 6:24 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 1424

View Options

libtaskotron/directives/bodhi_comment_directive.py

Show First 20 Lines • Show All 196 Lines • ▼ Show 20 Line(s)194 log.warning("Failed to post Bodhi Comment: `%s` `%s` `%s`",
197 detail.outcome) 197​ detail.outcome)
198 198​
199 output_data.append(outcome) 199​ output_data.append(outcome)
200 200​
201 # Posting to Bodhi takes a long time. We need to display some progress, 201​ # Posting to Bodhi takes a long time. We need to display some progress,
202 # else our tasks might get killed due to inactivity. Print status 202​ # else our tasks might get killed due to inactivity. Print status
203 # for every 20th build, or when complete. 203​ # for every 20th build, or when complete.
204 if index % 20 == 0 or index == len(check_details): 204​ if index % 20 == 0 or index == len(check_details):
205 log.info('Bodhi results submitted: %d/%d', index, 205​ log.info('Bodhi results processed: %d/%d', index,
206 len(check_details)) 206​ len(check_details))
207 207​
208 return output_data 208​ return output_data
209 209​
210 210​
211class BodhiUpdateState: 211​class BodhiUpdateState:
212 def __init__(self, update_id, kojitag='updates'): 212​ def __init__(self, update_id, kojitag='updates'):
213 self.test_states = { 'depcheck_32':'NORUN', 213​ self.test_states = { 'depcheck_32':'NORUN',
▲ Show 20 Lines • Show All 87 Lines • ▼ Show 20 Line(s)297 '''
301 :param str update_id: ID of the update to be tested 301​ :param str update_id: ID of the update to be tested
302 :param parsed_comments: already existing Taskotron comments for 302​ :param parsed_comments: already existing Taskotron comments for
303 the update 303​ the update
304 :type parsed_comments: list of dict 304​ :type parsed_comments: list of dict
305 :param dict new_result: the new result to be posted 305​ :param dict new_result: the new result to be posted
306 :param str kojitag: the koji tag being tested (affects the expected tests) 306​ :param str kojitag: the koji tag being tested (affects the expected tests)
307 :rtype: bool 307​ :rtype: bool
308 ''' 308​ '''
309​ # NOTE: Bodhi2 no longer supports forcing or supressing emails:
310​ # https://github.com/fedora-infra/bodhi/issues/289
311​ # (they might decide to change it, the ticket is not closed yet)
309 312​
310 # compute current state 313​ # compute current state
311 update_state = BodhiUpdateState(update_id, kojitag=kojitag) 314​ update_state = BodhiUpdateState(update_id, kojitag=kojitag)
312 for result in parsed_comments: 315​ for result in parsed_comments:
313 update_state.add_result(result) 316​ update_state.add_result(result)
314 317​
315 current_state = update_state.get_state() 318​ current_state = update_state.get_state()
316 log.debug('Current state for %s: %s\n %s', update_id, current_state, 319​ log.debug('Current state for %s: %s\n %s', update_id, current_state,
▲ Show 20 Lines • Show All 86 Lines • ▼ Show 20 Line(s)404 '''Check if the comment is meant to be posted.
403 :param str old_result: the result of the last test 406​ :param str old_result: the result of the last test
404 :param str comment_time: the comment time of the last test 407​ :param str comment_time: the comment time of the last test
405 :param str result: the result of the test 408​ :param str result: the result of the test
406 :param int time_span: waiting period (in minutes) before posting the same comment 409​ :param int time_span: waiting period (in minutes) before posting the same comment
407 410​
408 :return: ``True`` if the comment will be posted, ``False`` otherwise. 411​ :return: ``True`` if the comment will be posted, ``False`` otherwise.
409 :rtype: bool 412​ :rtype: bool
410 ''' 413​ '''
411 # NOTE: Bodhi2 no longer supports forcing or supressing emails: 414​ # we decided we will post just FAILED results, unless it's a change of state
412 # https://github.com/fedora-infra/bodhi/issues/289 415​ # (e.g. FAILED->PASSED)
413 # (they might decide to change it, the ticket is not closed yet) 416​
417​ if not old_result:
418​ return result == 'FAILED' # only post FAILED for the initial comment
414 419​
415 # the first comment or a comment with different result, post it 420​ if old_result != result: # change of state
416 if not old_result or old_result != result: ​
417 return True 421​ return True
418 422​
419 # If we got here, it means that the comment with the same result has been 423​ # If we got here, it means that the comment with the same result has been
420 # already posted, we now need to determine whether we can post the 424​ # already posted, we now need to determine whether we can post the
421 # comment again or not. 425​ # comment again or not.
422 # If the previous result is *not* 'FAILED', we won't post it in order not to 426​ # If the previous result is *not* 'FAILED', we won't post it in order not to
423 # spam developers. 427​ # spam developers.
424 # If the previous result *is* 'FAILED', we will need to check whether given 428​ # If the previous result *is* 'FAILED', we will need to check whether given
425 # time span expired, if so, we will post the same comment again to remind 429​ # time span expired, if so, we will post the same comment again to remind
426 # a developer about the issue. 430​ # a developer about the issue.
431​ assert old_result == result
427 432​
428 if result != 'FAILED': 433​ if result != 'FAILED':
429 return False 434​ return False
430 435​
431 if time_span is None: 436​ if time_span is None:
432 Config = config.get_config() 437​ Config = config.get_config()
433 time_span = Config.bodhi_posting_comments_span 438​ time_span = Config.bodhi_posting_comments_span
434 439​
▲ Show 20 Lines • Show All 54 Lines • ▼ Show 20 Line(s)492 if not bodhi_update:
489 # the TAP might not contain `item: updateid` but `item: update_title` 494​ # the TAP might not contain `item: updateid` but `item: update_title`
490 log.warning("No update with ID '%s' found, skipping...") 495​ log.warning("No update with ID '%s' found, skipping...")
491 return False 496​ return False
492 497​
493 (old_result, comment_time) = _already_commented(bodhi_update, testname, arch) 498​ (old_result, comment_time) = _already_commented(bodhi_update, testname, arch)
494 comment_needed = _is_comment_needed(old_result, comment_time, result) 499​ comment_needed = _is_comment_needed(old_result, comment_time, result)
495 500​
496 if not comment_needed and doreport != 'all': 501​ if not comment_needed and doreport != 'all':
497 log.debug('Up-to-date test result is already present in Bodhi: %s %s', 502​ log.debug('Bodhi comment is not needed for: %s %s',
498 updateid, result) 503​ updateid, result)
499 return True 504​ return True
500 505​
501 # submit the comment 506​ # submit the comment
502 bodhi_api.client.comment(bodhi_update['title'], comment, karma) 507​ bodhi_api.client.comment(bodhi_update['title'], comment, karma)
503 log.debug('Test result successfully submitted to Bodhi: %s %s', 508​ log.debug('Test result successfully submitted to Bodhi: %s %s',
504 updateid, result) 509​ updateid, result)
505 510​
506 except Exception as e: 511​ except Exception as e:
507 log.exception(e) 512​ log.exception(e)
508 return False 513​ return False
509 514​
510 return True 515​ return True
View Options

testing/test_bodhi_comment_directive.py

Show First 20 Lines • Show All 140 Lines • ▼ Show 20 Line(s)127 def test_already_commented(self):
141 update = deepcopy(self.update) 141​ update = deepcopy(self.update)
142 update['date_modified'] = "2014-01-31 15:00:00" 142​ update['date_modified'] = "2014-01-31 15:00:00"
143 outcome = bodhi_comment_directive._already_commented(update, 'depcheck', 'x86_64') 143​ outcome = bodhi_comment_directive._already_commented(update, 'depcheck', 'x86_64')
144 assert outcome == ('', '') 144​ assert outcome == ('', '')
145 145​
146 def test_is_comment_needed(self): 146​ def test_is_comment_needed(self):
147 """Checks whether _is_comment_needed returns proper results in 147​ """Checks whether _is_comment_needed returns proper results in
148 various situations.""" 148​ various situations."""
149 # Test first comment 149​ # Test first comment FAILED
150 outcome = bodhi_comment_directive._is_comment_needed(None, None, None, None) 150​ outcome = bodhi_comment_directive._is_comment_needed(None, None, 'FAILED', None)
151 assert outcome is True 151​ assert outcome is True
152 152​
153​ # Test first comment anything else
154​ outcome = bodhi_comment_directive._is_comment_needed(None, None, 'PASSED', None)
155​ assert outcome is False
156​
153 # Test different result 157​ # Test different result
154 outcome = bodhi_comment_directive._is_comment_needed('PASSED', None, 'FAILED', None) 158​ outcome = bodhi_comment_directive._is_comment_needed('PASSED', None, 'FAILED', None)
155 assert outcome is True 159​ assert outcome is True
160​ outcome = bodhi_comment_directive._is_comment_needed('FAILED', None, 'PASSED', None)
161​ assert outcome is True
156 162​
157 # Test same result, but not FAILED 163​ # Test same result, but not FAILED
158 outcome = bodhi_comment_directive._is_comment_needed('PASSED', None, 'PASSED', None) 164​ outcome = bodhi_comment_directive._is_comment_needed('PASSED', None, 'PASSED', None)
159 assert outcome is False 165​ assert outcome is False
160 166​
161 def test_is_comment_needed_decide_repost(self, monkeypatch): 167​ def test_is_comment_needed_decide_repost(self, monkeypatch):
162 """Checks whether time_span has an impact on comment posting.""" 168​ """Checks whether time_span has an impact on comment posting."""
163 date_utcnow = datetime.datetime(2014, 01, 10, 12, 05) 169​ date_utcnow = datetime.datetime(2014, 01, 10, 12, 05)
▲ Show 20 Lines • Show All 214 Lines • Show Last 20 Lines