RFR: 1565: Only poll for updated MRs from GitLab [v5]
Erik Joelsson
erikj at openjdk.org
Thu Sep 8 21:08:25 UTC 2022
On Thu, 8 Sep 2022 20:57:54 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
>> 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:
>
> Adding log message
Removing draft. This is now ready for review, but depends on SKARA-1589.
-------------
PR: https://git.openjdk.org/skara/pull/1369
More information about the skara-dev
mailing list