Skip to content

[CI] Use github's graphQL to query review state directly#14661

Merged
hail-ci-robot merged 5 commits intohail-is:mainfrom
cjllanwarne:cjl_github_review_state
Aug 25, 2024
Merged

[CI] Use github's graphQL to query review state directly#14661
hail-ci-robot merged 5 commits intohail-is:mainfrom
cjllanwarne:cjl_github_review_state

Conversation

@cjllanwarne
Copy link
Copy Markdown
Collaborator

@cjllanwarne cjllanwarne commented Aug 20, 2024

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:

Comment thread ci/ci/github.py Outdated
query {{
repository(owner: "{self.target_branch.branch.repo.owner}", name: "{self.target_branch.branch.repo.name}") {{
pullRequest(number: {self.number}) {{
number
Copy link
Copy Markdown
Member

@ehigham ehigham Aug 21, 2024

Choose a reason for hiding this comment

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

Are you using number?

Copy link
Copy Markdown
Collaborator Author

@cjllanwarne cjllanwarne Aug 21, 2024

Choose a reason for hiding this comment

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

Ooh, good catch. This is the PR number, I was just using it as a sanity check when testing the API out manually. It's not really necessary any more, assuming we trust github to return what we ask it for

Copy link
Copy Markdown
Contributor

@iris-garden iris-garden left a comment

Choose a reason for hiding this comment

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

Looks good to me with the manual testing!

Copy link
Copy Markdown
Member

@patrick-schultz patrick-schultz left a comment

Choose a reason for hiding this comment

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

Thanks Chris, this is clearly the right way to get the review state.

@cjllanwarne cjllanwarne force-pushed the cjl_github_review_state branch from f7a3230 to eee1702 Compare August 25, 2024 17:44
@hail-ci-robot hail-ci-robot merged commit 0b2087d into hail-is:main Aug 25, 2024
@cjllanwarne cjllanwarne deleted the cjl_github_review_state branch August 26, 2024 14:26
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.

CI considers PRs as merge candidates with only 1 review

5 participants