From 26e42a797bbac2a4b3454d304e6d150bcb77aca7 Mon Sep 17 00:00:00 2001 From: Chris Llanwarne Date: Tue, 20 Aug 2024 17:55:52 -0400 Subject: [PATCH 1/5] Use github's graphQL to query review state directly --- 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..a8b58e05a89 100644 --- a/ci/ci/github.py +++ b/ci/ci/github.py @@ -486,25 +486,34 @@ 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}) {{ + 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'Unexpected review decision: {review_decision} in PR {self.number}') + review_state = 'pending' if review_state != self.review_state: self.set_review_state(review_state) From f1abe9954f4d5ef23b5d7a31720ada721e4abef3 Mon Sep 17 00:00:00 2001 From: Chris Llanwarne Date: Wed, 21 Aug 2024 10:44:27 -0400 Subject: [PATCH 2/5] Add log line to make sure it's working and in analogy to update check state --- ci/ci/github.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/ci/github.py b/ci/ci/github.py index a8b58e05a89..4abe8dc6cca 100644 --- a/ci/ci/github.py +++ b/ci/ci/github.py @@ -516,6 +516,7 @@ async def _update_github_review_state(self, gh): 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 From f4f5f71783a05558c37007a38c02d566e81d5db6 Mon Sep 17 00:00:00 2001 From: Chris Llanwarne Date: Wed, 21 Aug 2024 11:54:32 -0400 Subject: [PATCH 3/5] More standard context prefix in log message --- ci/ci/github.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/ci/github.py b/ci/ci/github.py index 4abe8dc6cca..111974b7760 100644 --- a/ci/ci/github.py +++ b/ci/ci/github.py @@ -512,7 +512,7 @@ async def _update_github_review_state(self, gh): review_state = 'pending' else: # Should be impossible, per https://docs.github.com/en/graphql/reference/enums#pullrequestreviewdecision - log.error(f'Unexpected review decision: {review_decision} in PR {self.number}') + log.error(f'{self.short_str()}: unexpected review decision from github: {review_decision}') review_state = 'pending' if review_state != self.review_state: From a044b57bd76e350ff7129f535727b5d10eb21be4 Mon Sep 17 00:00:00 2001 From: Chris Llanwarne Date: Wed, 21 Aug 2024 13:33:47 -0400 Subject: [PATCH 4/5] Remove PR number from query --- ci/ci/github.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ci/ci/github.py b/ci/ci/github.py index 111974b7760..26028874df4 100644 --- a/ci/ci/github.py +++ b/ci/ci/github.py @@ -490,7 +490,6 @@ async def _update_github_review_state(self, gh): query {{ repository(owner: "{self.target_branch.branch.repo.owner}", name: "{self.target_branch.branch.repo.name}") {{ pullRequest(number: {self.number}) {{ - number reviewDecision }} }} From eee17021fb41edceb197afd26ba4bbc9607c7e6e Mon Sep 17 00:00:00 2001 From: Chris Llanwarne Date: Wed, 21 Aug 2024 13:35:08 -0400 Subject: [PATCH 5/5] Standardize on single quotes --- ci/ci/github.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/ci/github.py b/ci/ci/github.py index 26028874df4..bd8375bc9c9 100644 --- a/ci/ci/github.py +++ b/ci/ci/github.py @@ -497,7 +497,7 @@ async def _update_github_review_state(self, gh): """ response = await gh.post('/graphql', data={'query': review_state_query}) - review_decision = response["data"]["repository"]["pullRequest"]["reviewDecision"] + review_decision = response['data']['repository']['pullRequest']['reviewDecision'] if review_decision == 'APPROVED': review_state = 'approved'