Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make pull request assignment loading more robust #1918

Merged
merged 8 commits into from
Apr 10, 2025

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Mar 10, 2025

This PR reimplements the loading of open pull requests and their assignments to GitHub users. In particular, it changes the implementation from a GraphQL query to REST API calls, which is 5-10x faster based on my measurements, and should avoid the timeouts that I saw in production logs. It also performs this fetch periodically, every 30 minutes, so that we can periodically synchronize the latest state and thus paper over any potentially missed webhooks.

The initial state is also loaded eagerly when triagebot starts, so that we always have a consistent state of the world. If triagebot started without it, it could (in the future, once the workqueue is taken into account) for some time assign PRs based on the assumption that no reviewer has any assigned PRs, which is not great.

Best reviewed commit by commit.

The last two commits remove the GQL query, which is now unused, and also the one-off job mechanism, since the only job that was using it is now a cron job instead.

@Kobzol Kobzol requested review from apiraino and jackh726 March 10, 2025 17:39
@Kobzol Kobzol force-pushed the open-prs-refactor branch from 3871503 to 0d6967e Compare March 10, 2025 17:43
@apiraino apiraino removed their request for review March 13, 2025 12:42
@Kobzol Kobzol requested review from ehuss and removed request for ehuss March 18, 2025 22:18
@Kobzol Kobzol requested a review from Urgau April 7, 2025 19:03
Copy link
Member

@Urgau Urgau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started reviewing the PR, first 3 commits looks good, r=me on them.

I have a question on the fourth and will review the rest tomorrow.

Copy link
Member

@Urgau Urgau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good to me.

If you have confirmed that it works in practice, let's merge it.

@Kobzol Kobzol force-pushed the open-prs-refactor branch from 0d6967e to 9fdede1 Compare April 10, 2025 07:36
@Kobzol
Copy link
Contributor Author

Kobzol commented Apr 10, 2025

Well, it's hard to really confirm something with triagebot 😆 But it seems to be loading fine for me locally. I added a small log to give us information when the loading starts and ends.

@Kobzol Kobzol added this pull request to the merge queue Apr 10, 2025
Merged via the queue into rust-lang:master with commit 60ca7c9 Apr 10, 2025
3 checks passed
@Kobzol Kobzol deleted the open-prs-refactor branch April 10, 2025 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants