From b177361f20ee3c2c9c866c0945ecb38b18bf677e Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 15 Dec 2023 10:38:08 +0100 Subject: [PATCH] [IMP] forwardport: locking during creation of fw batch Not sure why I didn't think about it previously (in #357), but it would make sense for the entire batch creation to be atomic and thus under the same lock. The commit during forward porting also makes a lot less sense: it was a failed early attempt at resolving the problem by hoping we'd win the race with the webhook (commit before the webhook hit). By locking the PRs table to update, we actually resolved it. But since all that happens then is a few updates and then a commit by the cron itself (it commits per batch), it's probably good enough to leave the entire thing under the same lock. This means we lock out other interactions a bit longer, but since the span is still just the forward port of a single batch it should not be too much of an issue outside of post-freeze recovery thingie. --- forwardport/models/project.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/forwardport/models/project.py b/forwardport/models/project.py index 13def2bf..5aaf3a20 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -810,6 +810,7 @@ class PullRequests(models.Model): # one of the PRs in the batch fails is huge problem, though this loop # only concerns itself with the creation of the followup objects so... new_batch = self.browse(()) + self.env.cr.execute('LOCK runbot_merge_pull_requests IN SHARE MODE') for pr in self: owner, _ = pr.repository.fp_remote_target.split('/', 1) source = pr.source_id or pr @@ -821,7 +822,6 @@ class PullRequests(models.Model): ) title, body = re.match(r'(?P[^\n]+)\n*(?P<body>.*)', message, flags=re.DOTALL).groups() - self.env.cr.execute('LOCK runbot_merge_pull_requests IN SHARE MODE') r = gh.post(f'https://api.github.com/repos/{pr.repository.name}/pulls', json={ 'base': target.name, 'head': f'{owner}:{new_branch}', @@ -866,10 +866,6 @@ class PullRequests(models.Model): token_field='fp_github_token', format_args={'source': source, 'pr': pr, 'new': new_pr, 'footer': footer}, ) - # not great but we probably want to avoid the risk of the webhook - # creating the PR from under us. There's still a "hole" between - # the POST being executed on gh and the commit but... - self.env.cr.commit() for pr, new_pr in zip(self, new_batch): (h, out, err, hh) = conflicts.get(pr) or (None, None, None, None)