[IMP] runbot_merge: hide concurrent update errors

As far as I can tell they are properly handled:

- In `handle_status` we let the http layer retry the query, which
  pretty much always succeeds.
- In `Commit.notify`, we rollback the application of the current
  commit, meaning it'll be processed by the next run of the cron,
  which also seems to succeed every time (that is going through the
  log I pretty much never notice the same commit being serialization
  failure'd twice in a row).

  Which we can trigger for faster action, this last item is not
  entirely necessary as statuses should generally come in fast and
  especially if we have concurrency errors, but it can't hurt.

This means the only genuine issue is... sql_db logging a "bad query"
every time there's a serialization failure.

In `handle_status`, just suppress the message outright, if there's an
error other than serialization the http / dispatch layer should catch
and log it.

In `Commit._notify` things are slightly more difficult as the execute
is implicit (`flush` -> `_write` -> `execute`) so we can't pass the
flag by parameter. One option would be to set and unset
`_default_log_exception`, but it would either be a bit dodgy or it
would require using a context manager and increasing the indentation
level (or using a custom context manager).

Instead just `mute_logger` the fucking thing. It's a bit brutish and
mostly used in tests, but not just, and feels like the least bad
option here...

Closes #805
This commit is contained in:
Xavier Morel 2024-10-22 13:57:58 +02:00
parent 2174d7da31
commit 2fea318830
2 changed files with 9 additions and 4 deletions

View File

@ -374,7 +374,7 @@ def handle_status(env, event):
SET to_check = true,
statuses = c.statuses::jsonb || EXCLUDED.statuses::jsonb
WHERE NOT c.statuses::jsonb @> EXCLUDED.statuses::jsonb
""", [event['sha'], status_value])
""", [event['sha'], status_value], log_exceptions=False)
env.ref("runbot_merge.process_updated_commits")._trigger()
return 'ok'

View File

@ -23,7 +23,7 @@ from markupsafe import Markup
from odoo import api, fields, models, tools, Command
from odoo.exceptions import AccessError, UserError
from odoo.osv import expression
from odoo.tools import html_escape, Reverse
from odoo.tools import html_escape, Reverse, mute_logger
from . import commands
from .utils import enum, readonly, dfm
@ -1978,9 +1978,11 @@ class Commit(models.Model):
self.env.ref("runbot_merge.process_updated_commits")._trigger()
return r
@mute_logger('odoo.sql_db')
def _notify(self):
Stagings = self.env['runbot_merge.stagings']
PRs = self.env['runbot_merge.pull_requests']
serialization_failures = False
# chances are low that we'll have more than one commit
for c in self.search([('to_check', '=', True)]):
sha = c.sha
@ -1999,15 +2001,18 @@ class Commit(models.Model):
if stagings:
stagings._notify(c)
except psycopg2.errors.SerializationFailure:
_logger.info("Failed to apply commit %s (%s): serialization failure", c, sha)
serialization_failures = True
_logger.info("Failed to apply commit %s: serialization failure", sha)
self.env.cr.rollback()
except Exception:
_logger.exception("Failed to apply commit %s (%s)", c, sha)
_logger.exception("Failed to apply commit %s", sha)
self.env.cr.rollback()
else:
self.env.cr.precommit.data['change-message'] = \
f"statuses changed on {sha}"
self.env.cr.commit()
if serialization_failures:
self.env.ref("runbot_merge.process_updated_commits")._trigger()
_sql_constraints = [
('unique_sha', 'unique (sha)', 'no duplicated commit'),