RFR: 1565: Only poll for updated MRs from GitLab [v5]
duke at openjdk.org
Mon Sep 12 12:36:19 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
The dependent pull request has now been integrated, and the target branch of this pull request has been updated. This means that changes from the dependent pull request can start to show up as belonging to this pull request, which may be confusing for reviewers. To remedy this situation, simply merge the latest changes from the new target branch into this pull request by running commands similar to these in the local repository for your personal fork:
git checkout SKARA-1565-pr-poll
git fetch https://git.openjdk.org/skara master
git merge FETCH_HEAD
# if there are conflicts, follow the instructions given by git merge
git commit -m "Merge master"
More information about the skara-dev