Skip to content

Commit

Permalink
[CI] Use github's graphQL to query review state directly (#14661)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
cjllanwarne committed Aug 25, 2024
1 parent 882b1c7 commit 0b2087d
Showing 1 changed file with 28 additions and 19 deletions.
47 changes: 28 additions & 19 deletions ci/ci/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 0b2087d

Please sign in to comment.