fix(source-github): handle HTTP 200 GraphQL rate limits - #83256
Open
Misha Konovalov (mskonovalov) wants to merge 4 commits into
Open
fix(source-github): handle HTTP 200 GraphQL rate limits#83256Misha Konovalov (mskonovalov) wants to merge 4 commits into
Misha Konovalov (mskonovalov) wants to merge 4 commits into
Conversation
Contributor
👋 Welcome to Airbyte!Thank you for your contribution from mskonovalov/airbyte! We're excited to have you in the Airbyte community. If you have any questions, feel free to ask in the PR comments or join our Slack community. 💡 Show Tips and TricksPR Slash CommandsAs needed or by request, Airbyte Maintainers can execute the following slash commands on your PR:
Tips for Working with CI
📚 Show Repo GuidanceHelpful Resources
|
Misha Konovalov (mskonovalov)
marked this pull request as ready for review
July 30, 2026 07:42
Contributor
|
↪️ Triggering Reason: Community PR is ready for review with no AI review run yet; a review pass will identify which of the red connector checks are fork-credential artifacts versus real blockers. |
Contributor
Reviewing PR for connector safety and quality.
|
Contributor
🛡️ AI PR Review Report🔴 Review Action: REQUEST CHANGES
|
| Gate | Status | Enforced? | Details |
|---|---|---|---|
| PR Hygiene | FAIL | Yes | Description is substantial (1,300+ chars), but the connector has functional changes with no dockerImageTag bump and no docs/integrations/sources/github.md changelog entry. No unresolved human review comments. |
| Code Hygiene | PASS | WARNING | Source changed (errors_handlers.py, streams.py) and tests changed (unit_tests/test_stream.py, +45 lines). Coverage evidence: Found. |
| Test Coverage | PASS | Yes | Behavioral change (title fix(...)); new test test_graphql_rate_limit_uses_reset_backoff adds def test_..., @patch decorators and assert lines. |
| Code Security | PASS | Yes | No changed file matches auth/credential/token/secret path patterns; no security keywords added in diff hunks. |
| Per-Record Performance | PASS | WARNING | Change is confined to the HTTP error-resolution path, not a per-record loop; no new object creation per record. |
| Breaking Dependencies | PASS | WARNING | No pyproject.toml, lockfile, or Gradle dependency changes. |
| Backwards Compatibility | PASS | Yes (enforced) | No spec, metadata.yaml, stream schema, primary-key or cursor changes; nothing alters the discovered catalog type/format. |
| Forwards Compatibility | PASS | Warning (elevates Risk Level) | No state/cursor/partition/pagination/transformation keywords in added lines (the page_size lines in the hunk are unchanged context). |
| Behavioral Changes | FAIL | Warning (elevates Risk Level) | Added lines match rate-limit/retry keywords: ResponseAction.RATE_LIMITED in errors_handlers.py and error.get("type") in {"RATE_LIMIT", "RATE_LIMITED"} in streams.py. HTTP 200 GraphQL rate-limit responses now use reset-aware backoff instead of 5 short retries. Justified in the description and covered by a new unit test; warning-level only. |
| Out-of-Scope Changes | PASS | Skip | All 3 changed files are under airbyte-integrations/connectors/source-github/. |
| CI Checks | FAIL | Yes | Core check Test source-github Connector [No Creds] failed: 2 failed, 1 passed, 4 skipped in the standard suite due to missing secrets/config.json / config_oauth.json. Unit tests passed (163). Lint source-github Connector, Format Check, Build and Verify Artifacts (source-github), Check Changelog Updated all passed. source-github Pre-Release Checks and the Progressive Rollout / Rollout Checks Summary failures are excluded from this gate per playbook. |
| Live / E2E Tests | UNKNOWN | Yes | Validation is required (bug fix + API error-handling / sync-behavior change), and no /ai-prove-fix run exists on this PR. Not a FAIL — simply unvalidated. |
📚 Evidence Consulted
Evidence
- Changed files: 3
airbyte-integrations/connectors/source-github/source_github/errors_handlers.py(+6/-0)airbyte-integrations/connectors/source-github/source_github/streams.py(+1/-1)airbyte-integrations/connectors/source-github/unit_tests/test_stream.py(+45/-0)
- CI checks (HEAD
d0cd829): 22 passed, 4 failed. Failed:Test source-github Connector [No Creds],source-github Pre-Release Checks,source-github Progressive Rollout Gate,Connector Active Progressive Rollout Checks Summary. Passed (core):Lint source-github Connector,Format Check,Build and Verify Artifacts (source-github),Check Changelog Updated,Analyze Python,Enforce PR structure,Validate PR Title. - PR labels:
community,connectors/source/github,hyd-review(applied by this run). No validation/override labels. - PR description: present and detailed, including a production error trace and GitHub docs reference.
- Author justifications: none in the
## AI PR Review Justificationformat (the description does note that secret-backed standard tests and the rollout lookup cannot run on a fork PR — consistent with the observed CI failures). - Existing bot reviews: none for this HEAD SHA.
❓ How to Respond
Providing Context or Justification
You can add explanations that the bot will see on the next review:
Option 1: PR Description (recommended)
Add a section to your PR description:
## AI PR Review Justification
### {Gate Name}
[Your explanation here]Option 2: PR Comment
Add a comment starting with:
AI PR Review Justification:
[Your explanation here]
After adding your response, re-run /ai-review to have the bot evaluate it.
Note: Justifications provide context for the bot to evaluate. For some gates (like the Live / E2E Tests gate), a sufficient justification can lead to PASS. For other gates, justifications help explain the situation but may still require escalation if the gate cannot be remediated.
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
3 participants
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.
GitHub GraphQL can report exhausted quota with HTTP 200 and an error type of
RATE_LIMIT, while the connector only recognizes the olderRATE_LIMITEDform. The GraphQL-specific handler also handles generic response errors before the shared rate-limit handler, so this response receives five short retries and fails instead of waiting forX-RateLimit-Reset.GitHub documents that an exhausted primary GraphQL rate limit returns HTTP 200 with an error body and
x-ratelimit-remaining: 0, and that clients should wait untilx-ratelimit-resetbefore retrying. The exacterrors[].typevalue is not documented, so the connector must tolerate both observed variants.Observed during a production
reviewssync:This preserves generic GraphQL error retries while allowing both GitHub rate-limit variants to use Airbyte's reset-aware rate-limit handling.
Because this changes connector behavior, it is released as patch version 2.1.39.
Validation: CI passes unit tests, connector integration tests, lint, and format. Separate regression tests cover the existing
RATE_LIMITEDresponse and the productionRATE_LIMITpayload. The secret-backed standard tests and progressive-rollout lookup cannot run for this fork PR because GitHub does not providesecrets/config*.jsonor the rollout database credential.