From 0b2087db94a918514acfaa1342de474f73a81bd9 Mon Sep 17 00:00:00 2001 From: Chris Llanwarne Date: Sun, 25 Aug 2024 14:28:17 -0400 Subject: [PATCH] [CI] Use github's graphQL to query review state directly (#14661) Fixes #14660 by using the graphQL API to query github directly. Replaces our current parallel interpretation of reviews into a review decision, which is brittle if we ever change review requirements in github again. Tested by manually updating the live CI to use the test batch generated image. Results: - Review decisions correctly fetched from github, not based on CI's parallel interpretation of individual reviews: ![image](https://github.com/user-attachments/assets/67c03aa9-000a-44e7-91aa-3a42d04238dc) - No merge candidate was being incorrectly nominated (in particular, #14645 is now considered pending, rather than approved, which is what we are currently, incorrectly, calculating) --- ci/ci/github.py | 47 ++++++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/ci/ci/github.py b/ci/ci/github.py index 8adabce26fe..bd8375bc9c9 100644 --- a/ci/ci/github.py +++ b/ci/ci/github.py @@ -486,27 +486,36 @@ async def _update_last_known_github_status(self, gh): self.target_branch.state_changed = True async def _update_github_review_state(self, gh): - latest_state_by_login = {} - async for review in gh.getiter( - f'/repos/{self.target_branch.branch.repo.short_str()}/pulls/{self.number}/reviews' - ): - login = review['user']['login'] - state = review['state'] - # reviews is chronological, so later ones are newer statuses - if state != 'COMMENTED': - latest_state_by_login[login] = state - - review_state = 'pending' - for login, state in latest_state_by_login.items(): - if state == 'CHANGES_REQUESTED': - review_state = 'changes_requested' - break - if state == 'APPROVED': - review_state = 'approved' - else: - assert state in ('DISMISSED', 'COMMENTED', 'PENDING'), state + review_state_query = f""" + query {{ + repository(owner: "{self.target_branch.branch.repo.owner}", name: "{self.target_branch.branch.repo.name}") {{ + pullRequest(number: {self.number}) {{ + reviewDecision + }} + }} + }} + """ + + response = await gh.post('/graphql', data={'query': review_state_query}) + review_decision = response['data']['repository']['pullRequest']['reviewDecision'] + + if review_decision == 'APPROVED': + review_state = 'approved' + elif review_decision == 'CHANGES_REQUESTED': + review_state = 'changes_requested' + elif review_decision == 'REVIEW_REQUIRED': + review_state = 'pending' + elif review_decision is None: + # This probably means the repo has no "required reviews" configuration. But CI shouldn't merge without + # at least one approval, so we'll treat this as "pending": + review_state = 'pending' + else: + # Should be impossible, per https://docs.github.com/en/graphql/reference/enums#pullrequestreviewdecision + log.error(f'{self.short_str()}: unexpected review decision from github: {review_decision}') + review_state = 'pending' if review_state != self.review_state: + log.info(f'{self.short_str()}: review state changing from {self.review_state} => {review_state}') self.set_review_state(review_state) self.target_branch.state_changed = True