dotnet / issue-labeler

An issue labeler bot for use in dotnet repositories.
MIT License
22 stars 20 forks source link

Address race condition with issue-labeler overwriting labels added by other processes #43

Open maryamariyan opened 2 years ago

maryamariyan commented 2 years ago
image

As seen in https://github.com/dotnet/runtime/issues/69428, shows that msft bot and dotnet-issue-labeler bot are simultaneously trying to update the labels for the issue and the last one wins.

One possible solution here is to allow adding a configurable delay amount after the issue is created and before the dotnet-issue-labeler is allowed to update the labels, to give msft bot enough time to complete its labeling.

/cc @eerhardt @jeffhandley

maryamariyan commented 2 years ago

This issue is caused by this recent change: https://github.com/dotnet/issue-labeler/pull/39

maryamariyan commented 2 years ago

Seems like we already have configuration to allow delay.

https://github.com/dotnet/issue-labeler/blob/093a4e7ea471dd9decb8e9ba952718b2cd445d23/src/Microsoft.DotNet.GitHub.IssueLabeler/Models/Labeler.cs#L185-L189

Maybe the delay time needs to get updated

maryamariyan commented 2 years ago

Alternatively we could test if this REST API can be used instead to avoid the race condition

https://docs.github.com/en/rest/issues/labels#add-labels-to-an-issue

ericstj commented 2 years ago

@eiriktsarpalis noticed the same issue here https://github.com/dotnet/runtime/issues/69320#event-6607257486

He suggested the approach described here: https://docs.github.com/en/rest/overview/resources-in-the-rest-api#conditional-requests

It's not clear to me how one might use this with OctoKit, it looks to me like we eventually go through this API in OctoKit: https://github.com/dotnet/issue-labeler/blob/093a4e7ea471dd9decb8e9ba952718b2cd445d23/src/Microsoft.DotNet.GitHub.IssueLabeler/Models/Labeler.cs#L621 https://github.com/octokit/octokit.net/blob/a5afcc229fea93644004fcbd434a5cc2e8865a21/Octokit/Clients/IssuesClient.cs#L496-L501

jeffhandley commented 2 years ago

We could potentially issue a GraphQL mutation request here instead too. The GraphQL APIs have the ability to add a label to an issue atomically, without affecting other labels.

jeffhandley commented 2 years ago

FYI -- I've reviewed all dotnet/runtime issues filed between May 2 and June 24 and made corrective actions where the untriaged label was removed due to this race condition. There were about a dozen affected issues.