From 0334225114fd650cfaab54a96125f6f024be0326 Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Mon, 5 Aug 2024 15:57:06 +0200
Subject: [PATCH] [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
---
 runbot_merge/models/pull_requests.py | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py
index 2901def0..c2b243bb 100644
--- a/runbot_merge/models/pull_requests.py
+++ b/runbot_merge/models/pull_requests.py
@@ -13,7 +13,7 @@ from functools import reduce
 from operator import itemgetter
 from typing import Optional, Union, List, Iterator, Tuple
 
-import psycopg2
+import psycopg2.errors
 import sentry_sdk
 import werkzeug
 from markupsafe import Markup
@@ -1934,25 +1934,30 @@ class Commit(models.Model):
         PRs = self.env['runbot_merge.pull_requests']
         # chances are low that we'll have more than one commit
         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:
                 c.to_check = False
-                pr = PRs.search([('head', '=', c.sha)])
+                self.flush(['to_check'], c)
                 if pr:
-                    self.env.cr.precommit.data['change-message'] =\
-                        f"statuses changed on {c.sha}"
                     pr._validate(c.statuses)
 
-                stagings = Stagings.search([
-                    ('head_ids.sha', '=', c.sha),
-                    ('state', '=', 'pending'),
-                    ('target.project_id.staging_statuses', '=', True),
-                ])
                 if stagings:
                     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:
-                _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()
             else:
+                self.env.cr.precommit.data['change-message'] = \
+                    f"statuses changed on {sha}"
                 self.env.cr.commit()
 
     _sql_constraints = [