RFR: 2189: Mark command as handled when JCheckConfiguration is missing or invalid in PullRequestCommandWorkItem

[Erik Joelsson]

On Fri, 1 Mar 2024 23:11:57 GMT, Zhao Song <zsong at openjdk.org> wrote:

> Recently, we found that the skara bot keeps trying to process /open command in this pr https://github.com/openjdk/jdk8u-dev/pull/393
> 
> After investigation, we found that this pr is a dependent pr and this pr was closed before the parent pr(https://github.com/openjdk/jdk8u-dev/pull/392) got integrated, so this dependent pr didn't get retargeted.
> 
> Skara bot was trying to process the command but the bot couldn't find jcheck configuration in the target branch because the target is already deleted. And when the jcheck configuration is invalid or missing in PullRequestCommandWorkItem, RuntimeException would be thrown, causing the bot to repeatedly process the command. This behavior is wrong because when jcheck configuration is missing or invalid, it wouldn't recover automatically. In this case, the bot should just notice the user what problem it found and mark the command as handled to prevent the bot from being stuck in a loop.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestCommandWorkItem.java line 219:

> 217:                 var branchNames = pr.repository().branches().stream().map(HostedBranch::name).toList();
> 218:                 if (branchNames.contains(pr.targetRef())) {
> 219:                     printer.println("JCheck configuration is missing or invalid in the target branch of this pull request.");

We do know if it's missing or invalid based on the type of the exception. I think it's worth differentiating the message based on this.

bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestCommandWorkItem.java line 227:

> 225:                         " is missing or invalid in repo " + bot.confOverrideRepository().get().name());
> 226:                 printer.println("The JCheck configuration has been overridden, " +
> 227:                         "but it's missing or invalid. Skara admin has been noticed and will fix it as soon as possible.");

Suggestion:

                        "but is missing or invalid. Skara admins have been notified.");

bots/pr/src/main/java/org/openjdk/skara/bots/pr/PullRequestCommandWorkItem.java line 229:

> 227:                         "but it's missing or invalid. Skara admin has been noticed and will fix it as soon as possible.");
> 228:             }
> 229:             printer.print("Please issue this command again once the problem has been resolved.");

I don't think this should be printed on a separate line.

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

PR Review Comment: https://git.openjdk.org/skara/pull/1616#discussion_r1511325162
PR Review Comment: https://git.openjdk.org/skara/pull/1616#discussion_r1511322543
PR Review Comment: https://git.openjdk.org/skara/pull/1616#discussion_r1511326549