[FIX] runbot_merge: read sha first in commit statuses broadcast

Because the first operation the notification task performs is updating
the commit, this has to be flushed in order to read-back the commit
hash (probably fixed in later version).

This means if updating the flag triggers a serialization
failure (which happens and is normal) we transition to the `exception`
handler, which *tries to retrieve the sha again* and we get an "in
failed transaction" error on top of the current "serialization
failure", leading to a giant traceback.

Also an unhelpful one since a serialization failure is expected in
this cron.

- Move the reads with no write dependency out of the `try`, there's no
  reason for them to fail, and notably memoize the `sha`.
- Split the handling of serialization failures out of the normal one,
  and just log an `info` (we could actually log nothing, probably).
- Also set the precommit data just before the commit, in case staging
  tracking is ever a thing which could use it.

Fixes #909
This commit is contained in:
Xavier Morel 2024-08-05 15:57:06 +02:00
parent 52f2b1e381
commit 0334225114

View File

@ -13,7 +13,7 @@ from functools import reduce
from operator import itemgetter from operator import itemgetter
from typing import Optional, Union, List, Iterator, Tuple from typing import Optional, Union, List, Iterator, Tuple
import psycopg2 import psycopg2.errors
import sentry_sdk import sentry_sdk
import werkzeug import werkzeug
from markupsafe import Markup from markupsafe import Markup
@ -1934,25 +1934,30 @@ class Commit(models.Model):
PRs = self.env['runbot_merge.pull_requests'] PRs = self.env['runbot_merge.pull_requests']
# chances are low that we'll have more than one commit # chances are low that we'll have more than one commit
for c in self.search([('to_check', '=', True)]): for c in self.search([('to_check', '=', True)]):
sha = c.sha
pr = PRs.search([('head', '=', sha)])
stagings = Stagings.search([
('head_ids.sha', '=', sha),
('state', '=', 'pending'),
('target.project_id.staging_statuses', '=', True),
])
try: try:
c.to_check = False c.to_check = False
pr = PRs.search([('head', '=', c.sha)]) self.flush(['to_check'], c)
if pr: if pr:
self.env.cr.precommit.data['change-message'] =\
f"statuses changed on {c.sha}"
pr._validate(c.statuses) pr._validate(c.statuses)
stagings = Stagings.search([
('head_ids.sha', '=', c.sha),
('state', '=', 'pending'),
('target.project_id.staging_statuses', '=', True),
])
if stagings: if stagings:
stagings._notify(c) stagings._notify(c)
except psycopg2.errors.SerializationFailure:
_logger.info("Failed to apply commit %s (%s): serialization failure", c, sha)
self.env.cr.rollback()
except Exception: except Exception:
_logger.exception("Failed to apply commit %s (%s)", c, c.sha) _logger.exception("Failed to apply commit %s (%s)", c, sha)
self.env.cr.rollback() self.env.cr.rollback()
else: else:
self.env.cr.precommit.data['change-message'] = \
f"statuses changed on {sha}"
self.env.cr.commit() self.env.cr.commit()
_sql_constraints = [ _sql_constraints = [