RFR: 2086: Use short Ids to get cached issue data in CheckWorkItem#getIssueMetadata

Erik Joelsson erikj at openjdk.org
Mon Oct 30 23:51:05 UTC 2023


On Mon, 30 Oct 2023 21:22:29 GMT, Zhao Song <zsong at openjdk.org> wrote:

> A user reported that in a repo which is configured with maintainer approval feature, he added the approved label to the issue but skara bot didn't update the related PR.
> 
> From the log, I found skara bot said "[Issue]No activity since last check, not checking again."
> 
> When investigating, I guess the scenario likes this. When the bot is evaluating a PR. The bot fetches the issue and the issue only contains label “XXX-fix-request”. Then, the user add "XXX-fix-yes" to issue. So in the end of evaluation, the bot fetches the issue again and calculates the metadata again (at that time, the issue contains label “XXX-fix-request, XXX-fix-yes”). So in the next round, the bot would think that new label “XXX-fix-yes” is already handled, so it will not evaluate the PR again.
> 
> But as Erik introduced a cache for IssueTrackerIssue in CheckWorkItem in [SKARA-1963](https://bugs.openjdk.org/browse/SKARA-1963). I think the bot shouldn't fetch the issue from JBS again in the end of checkRun. But the bot really did. Then I found that the key of the map(cache) are supposed to be short id of the JBS issue, But in CheckWorkItem#getIssueMetadata, the bot is trying to use the whole id of JBS issue to get the cached data, which will always miss and trigger the remote call.

Can you add a comment on the `issueTrackerIssue` method that states that we expect the shortID, and perhaps also change the name of the parameter.

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

PR Review: https://git.openjdk.org/skara/pull/1579#pullrequestreview-1705238418


More information about the skara-dev mailing list