[ci] Resolve slow github polling and hook handling errors#15493
Open
cjllanwarne wants to merge 11 commits into
Open
[ci] Resolve slow github polling and hook handling errors#15493cjllanwarne wants to merge 11 commits into
cjllanwarne wants to merge 11 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the CI service’s GitHub integration to reduce polling latency by batching PR review/check retrieval into a single (paginated) GraphQL query, and fixes an error case in the deploy status endpoint when no deploy batch exists yet.
Changes:
- Replace per-PR GraphQL polling with
_fetch_pr_github_data, a batched GraphQL query that fetches reviewDecision + statusCheckRollup contexts for all open PRs at once (with pagination support). - Refactor PR GitHub state application into
PR._apply_github_dataand update WatchedBranch’s GitHub update flow to use the batched results (including additional summary logging). - Add unit tests and pytest configuration for async tests; adjust deploy status reporting to avoid calling failure-log collection when no deploy batch exists.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
ci/unit-test/test_github_unit.py |
Adds unit tests for batched GraphQL polling and for applying GitHub review/check results onto PR state. |
ci/unit-test/pytest.ini |
Enables pytest asyncio auto mode so async tests run without explicit markers/fixtures. |
ci/unit-test/conftest.py |
Provides safe env/config defaults and mocks global-config secret reading for local/unit-test execution. |
ci/ci/github.py |
Implements batched GraphQL PR polling and refactors PR status application; updates WatchedBranch GitHub update to use the new batching. |
ci/ci/ci.py |
Fixes deploy status endpoint to avoid errors when deploy batch/failure info isn’t available yet. |
Comments suppressed due to low confidence (1)
ci/ci/github.py:472
CheckRun.conclusionfrom the GitHub GraphQL API can be null for in-progress checks. Passing that null through togithub_statuswill raiseValueErrorand can break the watched-branch update loop. Consider treating a null conclusion as a pending status (or explicitly mapping null ->GithubStatus.PENDING) before callinggithub_status.
if (typename := check["__typename"]) == "StatusContext":
last_known_github_status[check["context"]] = github_status(check["state"])
elif typename == "CheckRun":
last_known_github_status[check["name"]] = github_status(check["conclusion"])
else:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+21
to
+37
| if not os.path.exists('/global-config'): | ||
| # Patch gear.cloud_config.read_config_secret so that ci.environment's module-level | ||
| # get_global_config() call (and the subsequent get_gcp_config() call) succeed locally. | ||
| _fake_global_config = { | ||
| 'cloud': 'gcp', | ||
| 'docker_prefix': 'gcr.io/hail-vdc', | ||
| 'docker_root_image': 'ubuntu:22.04', | ||
| 'domain': 'hail.is', | ||
| 'kubernetes_server_url': 'https://k8s.example.com', | ||
| 'default_namespace': 'default', | ||
| # Fields required by GCPConfig.from_global_config | ||
| 'batch_gcp_regions': json.dumps(['us-central1']), | ||
| 'gcp_region': 'us-central1', | ||
| 'gcp_project': 'hail-vdc', | ||
| 'gcp_zone': 'us-central1-a', | ||
| } | ||
| unittest.mock.patch('gear.cloud_config.read_config_secret', return_value=_fake_global_config).start() |
Comment on lines
+687
to
+693
| async def _fetch_pr_github_data( | ||
| gh: _GitHubGraphQL, | ||
| owner: str, | ||
| repo_name: str, | ||
| pr_numbers: List[int], | ||
| ) -> Dict[int, tuple]: | ||
| """Fetch review decisions and status check rollup for all PRs in a single batched GraphQL query. |
Collaborator
Author
There was a problem hiding this comment.
skipping this one
sarahgibs
approved these changes
May 21, 2026
kush-chandra
previously approved these changes
Jun 1, 2026
cjllanwarne
force-pushed
the
cjl_ci_github_broken
branch
from
9acd8d6 to
9eea584
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change Description
Two main changes:
Additionally, sets up a test framework in CI that can mock out GH to test the polling logic, and provide default environment variables during test runs so that we don't fail because of "global config missing" errors.
Security Assessment
Impact Rating
Impact Description
Small internal changes in how we poll github and how we manage internal state when we get state update hook calls.
Appsec Review