[FIX] runbot_merge: status deduplication (maybe)

Despite the existing dedup' sometimes the "xxx failed on this
forward-port PR" would still get multiplicated due to split builds
e.g. in odoo/odoo#43935 4 such messages appear within ~5 minutes, then
one more 10mn later.

This is despite all of them having the same "build" (target_url) and
status (failure). Since the description is the only thing that's not
logged I assume that's the field which varies and makes the dedup'
fail. Therefore:

* add the description to the logging (when getting a status ping)
* exclude the description when checking if a new status should be
  taken in account or ignored: the build (and thus url) should change
  on rebuild

Hopefully fixes #281
This commit is contained in:
Xavier Morel 2020-01-31 10:36:01 +01:00
parent e267533513
commit 35dbfd2c12
3 changed files with 14 additions and 2 deletions

View File

@ -261,6 +261,9 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
with prod:
prod.post_status(pr1.head, st, ctx)
env.run_crons()
with prod: # should be ignored because the description doesn't matter
prod.post_status(pr1.head, 'failure', 'ci/runbot', description="HAHAHAHAHA")
env.run_crons()
# check that FP did not resume & we have a ping on the PR
assert env['runbot_merge.pull_requests'].search([], order='number') == pr0 | pr1,\
"forward port should not continue on CI failure"

View File

@ -207,7 +207,7 @@ def handle_pr(env, event):
def handle_status(env, event):
_logger.info(
'status %(context)s:%(state)s on commit %(sha)s (%(target_url)s)',
'status on %(sha)s %(context)s:%(state)s (%(target_url)s) [%(description)r]',
event
)
Commits = env['runbot_merge.commit']

View File

@ -879,10 +879,19 @@ class PullRequests(models.Model):
# only sending notification if the newly failed status is different than
# the old one
prev = json.loads(self.previous_failure)
if st != prev:
if not self._statuses_equivalent(st, prev):
self.previous_failure = json.dumps(st)
self._notify_ci_failed(ci)
def _statuses_equivalent(self, a, b):
""" Check if two statuses are *equivalent* meaning the description field
is ignored (check only state and target_url). This is because the
description seems to vary even if the rest does not, and generates
unnecessary notififcations as a result
"""
return a.get('state') == b.get('state') \
and a.get('target_url') == b.get('target_url')
def _notify_ci_failed(self, ci):
# only report an issue of the PR is already approved (r+'d)
if self.state == 'approved':