RFR: 1565: Only poll for updated MRs from GitLab [v7]

Erik Joelsson erikj at openjdk.org
Mon Sep 12 13:52:10 UTC 2022


> This is another rather big patch, but again, with some explanation, it shouldn't be too scary. The most important part is the new `PullRequestPoller` class. This class implements a polling mechanism for PullRequests that tries to only return new or updated pull requests to a bot and guaranteeing that none are missed. The goal is to minimize unnecessary evaluations that currently take place, especially when polling Gitlab. I have added quite a few tests to verify (almost) all the functionality of the poller. For a more detailed explanation of all the challenges this poller faces, see the bug description.
> 
> In order to reduce impact, I'm only making one existing bot use the new poller, the CSRPullRequestBot. I picked this one as it should have pretty low impact overall if something goes wrong. I will followup with further conversions of other bots. I'm also touching the `CSRIssueBot` to introduce the same kind of error handling with retry that the new `PullRequestPoller` implements. I hadn't realized before that this error handling was missing in the CSRBots, and to be able to implement it properly for the `CSRPullRequestBot`, I had to also add the support for the issue polling `CSRIssueBot`.
> 
> There are features of the poller that aren't in use yet. I tried to look through all future users to make sure I could cover all existing usecases from the start.
> 
> Most of the rest of the changes are just adding `equals` and `hashCode` to a bunch of classes that we now need to compare (to know if a PR has been updated or not). This includes the `GitHubPullrequest`, `GitLabMergeRequest` and all the JSON types. For test code this also includes the `TestPullRequest` and `TestIssue` and their related data classes.
> 
> In order to test this properly, I needed to implement a way to for the `TestHostedRepository` to return copies of `TestPullRequest` instances from queries. This better simulates how new instances are always created when `PullRequest`s are queried from a remote `Forge`. The existing bot tests don't need it, but without this, it's not possible to verify the behavior of the poller. I tried a lot of different ways of implementing clone/copy (Object.clone(), copy constructor and specialized copy methods), but what I have here is what I think worked best in the end, with the least impact on existing code.

Erik Joelsson has updated the pull request incrementally with one additional commit since the last revision:

  Revert CSRBot, moving it to separate change

-------------

Changes:
  - all: https://git.openjdk.org/skara/pull/1369/files
  - new: https://git.openjdk.org/skara/pull/1369/files/217e1911..c83197fb

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=skara&pr=1369&range=06
 - incr: https://webrevs.openjdk.org/?repo=skara&pr=1369&range=05-06

  Stats: 114 lines in 4 files changed: 36 ins; 59 del; 19 mod
  Patch: https://git.openjdk.org/skara/pull/1369.diff
  Fetch: git fetch https://git.openjdk.org/skara pull/1369/head:pull/1369

PR: https://git.openjdk.org/skara/pull/1369


More information about the skara-dev mailing list