RFR: 2043: Skara mistakenly tagged the wrong commit when processing a /tag command
Zhao Song
zsong at openjdk.org
Thu Sep 28 21:23:18 UTC 2023
On Thu, 28 Sep 2023 18:17:29 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
> The method `GitLabRepository::recentCommitComments` may sometimes return a comment associated with the wrong commit. We have seen it happen, which caused a tag to be set on the wrong commit. My theory is that this was caused by a race, which was probably made more potent due to GitLab sometimes being slow in returning up to date data. See bug for more details.
>
> The main fix is to always verify that an event note about a recent commit comment actually matches a comment found on the commit in question. The current implementation has a quick return when only one potential commit is found.
>
> In addition to that, I'm also reworking the implementation to be a bit more robust. @zhaosongzs found that there is an alternate API for retrieving commit comments called "discussions" and that gives us more data, notably the same ID field as an event has. This makes it possible to match an event note with a commit comment note by ID instead of by the combination of author and created_at. From this further simplifications naturally fell out.
>
> A CommitComment returned from `GitLabRepository::addCommitComment` no longer contains a value for ID. I think this is more correct than making up a value. I verified that no code seems to be using the returned object currently, so this shouldn't impact anything.
>
> The two new tests were run and inspected manually.
Looks good.
forge/src/main/java/org/openjdk/skara/forge/gitlab/GitLabRepository.java line 557:
> 555: log.warning("Did not find commit with title " + commitTitle + " for repository " + projectName);
> 556: }
> 557: return found;
Just want to confirm that if we stop throwing exceptions here, the commitComment will be handled again in the next CommitCommentsWorkItem.
-------------
Marked as reviewed by zsong (Reviewer).
PR Review: https://git.openjdk.org/skara/pull/1564#pullrequestreview-1649728141
PR Review Comment: https://git.openjdk.org/skara/pull/1564#discussion_r1340678580
More information about the skara-dev
mailing list