Bound the GitHub client retries so rate limits fail fast (#8506)

Stack from [ghstack](https://github.com/ezyang/ghstack/tree/0.14.0)
(oldest at bottom):
* #8568
* #8508
* #8507
* __->__ #8506
* #8505

**Impact:** greenlight scan, fingerprint, and verdict paths (the GitHub
client)
**Risk:** low

## What
Wire a bounded plain `urllib3` `Retry` into `build_client`: 5xx-only
forcelist
(403/429 excluded), `respect_retry_after_header=False`, idempotent
methods only
(`GET`/`HEAD`/`PUT`/`DELETE`), `total=2`.

## Why
PyGithub's default `GithubRetry` force-lists 403 and sleeps in-call
until the
rate-limit reset (up to ~1h, unclampable by config), on fingerprint
worker
threads the main-thread SIGALRM timeout cannot interrupt -- so a single
scan
iteration could stall past the runtime budget and be force-killed
(daemon
watchdog / Lambda timeout). A 5xx-only retry that never honors
`Retry-After`
lets a rate limit raise immediately, so the pass is abandoned through
the
existing exception flow instead of hanging.

# Notes
Not retrying `POST`/`PATCH` also removes a latent double-review /
double-dispatch
hazard from the old default. Retry semantics are asserted in new
`test_github_client.py` cases, including that the type is a plain
`urllib3.Retry`
(not `GithubRetry`) and that 403/429 are excluded from the forcelist.

Signed-off-by: Jean Schmidt <contato@jschmidt.me>

Author: jeanschmidt

greenlight/src/greenlight/github_client.py

@@ -20,6 +20,7 @@
     from datetime import datetime
 
     from github import Github
+    from urllib3.util.retry import Retry
 
     from greenlight.github_types import (
         ScanClient,
@@ -49,11 +50,38 @@ class OpenPR:
 # worst-case pagination outlast the per-iteration runtime watchdog.
 _GITHUB_TIMEOUT_SECONDS: int = 15
 
+_GITHUB_RETRY_TOTAL: int = 2
+_GITHUB_RETRY_BACKOFF_FACTOR: float = 0.5
+_GITHUB_RETRY_BACKOFF_MAX_SECONDS: float = 5.0
+
+
+def _build_retry() -> Retry:
+    # Not PyGithub's default GithubRetry: it force-lists 403 and sleeps in-call until the
+    # rate-limit reset, which would stall a fingerprint worker past the per-iteration runtime
+    # budget. A plain urllib3 Retry with a 5xx-only forcelist lets a rate limit raise at once.
+    from urllib3.util.retry import Retry
+
+    return Retry(
+        total=_GITHUB_RETRY_TOTAL,
+        backoff_factor=_GITHUB_RETRY_BACKOFF_FACTOR,
+        backoff_max=_GITHUB_RETRY_BACKOFF_MAX_SECONDS,
+        status_forcelist=frozenset(range(500, 600)),
+        allowed_methods=frozenset({"GET", "HEAD", "PUT", "DELETE"}),
+        respect_retry_after_header=False,
+        raise_on_status=True,
+    )
+
 
 def build_client(token: str) -> Github:
     from github import Auth, Github  # lazy: keeps this module importable without the dep
 
-    return Github(auth=Auth.Token(token), per_page=100, timeout=_GITHUB_TIMEOUT_SECONDS, lazy=True)
+    return Github(
+        auth=Auth.Token(token),
+        per_page=100,
+        timeout=_GITHUB_TIMEOUT_SECONDS,
+        retry=_build_retry(),
+        lazy=True,
+    )
 
 
 def list_open_prs_by_authors(client: _RepoClient, repo: str, authors: Iterable[str]) -> list[OpenPR]:

greenlight/tests/test_github_client.py

@@ -2,6 +2,7 @@
 
 import pytest
 from github import Github
+from urllib3.util.retry import Retry
 
 from greenlight import github_client
 from greenlight.constants import EVAL_HASH_RE
@@ -365,6 +366,46 @@ def test_build_client_pins_request_timeout():
     assert requester.__dict__["_Requester__timeout"] == 15
 
 
+def test_build_retry_is_bounded_and_omits_rate_limit_statuses():
+    retry = github_client._build_retry()
+
+    # GithubRetry subclasses urllib3.Retry, so assert the exact type -- isinstance would
+    # pass for the GithubRetry we're rejecting.
+    assert type(retry) is Retry
+    assert 403 not in retry.status_forcelist
+    assert 429 not in retry.status_forcelist
+    assert 500 in retry.status_forcelist
+    assert 503 in retry.status_forcelist
+    assert retry.respect_retry_after_header is False
+    assert retry.allowed_methods == frozenset({"GET", "HEAD", "PUT", "DELETE"})
+    assert "PUT" in retry.allowed_methods
+    assert "DELETE" in retry.allowed_methods
+    assert "POST" not in retry.allowed_methods
+    assert "PATCH" not in retry.allowed_methods
+    assert retry.total == 2
+    assert retry.backoff_factor == 0.5
+    assert retry.backoff_max == 5.0
+
+
+def test_build_client_wires_bounded_retry_into_github(monkeypatch):
+    captured: dict[str, object] = {}
+
+    def _fake_github(**kwargs: object) -> object:
+        captured.update(kwargs)
+        return object()
+
+    # build_client re-runs `from github import ... Github` per call, so patching the attribute on
+    # the github module is picked up; Auth is left real (Auth.Token is a pure, offline wrapper).
+    monkeypatch.setattr("github.Github", _fake_github)
+
+    github_client.build_client("tok")
+
+    retry = captured["retry"]
+    assert isinstance(retry, Retry)
+    assert 403 not in retry.status_forcelist
+    assert retry.respect_retry_after_header is False
+
+
 def _build_fp(
     pr: _FakePR,
     *,