-
Notifications
You must be signed in to change notification settings - Fork 246
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
[CI] Use github's graphQL to query review state directly #14661
[CI] Use github's graphQL to query review state directly #14661
Conversation
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you using number
?
There was a problem hiding this comment.
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
There was a problem hiding this 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!
There was a problem hiding this 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.
f7a3230
to
eee1702
Compare
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:
job_group_inst_coll_cancellable_resources
#14645 is now considered pending, rather than approved, which is what we are currently, incorrectly, calculating)