From 08e14173cb98df5448641f7d2047d17c252a5868 Mon Sep 17 00:00:00 2001 From: autodev Date: Sat, 10 Oct 2026 18:16:47 -0400 Subject: [PATCH] autodev: conditional GitHub requests (ETags) for per-poll PR reads Co-Authored-By: Claude Sonnet 5.5 --- autodev/github.py | 13 +++++++------ tests/test_autodev_activities.py | 33 ++++++++++++++++++++++++++++++++ zarr_doctor/github_api.py | 24 ++++++++++++++++++----- 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/autodev/github.py b/autodev/github.py index eb9bb1d..2a121ae 100644 --- a/autodev/github.py +++ b/autodev/github.py @@ -35,7 +35,7 @@ async def label_actor(self, issue: int, label: str) -> str | None: return actor async def issue_comments(self, number: int) -> list[dict]: - return [c async for c in self._paged(f"/repos/{self.repo}/issues/{number}/comments")] + return [c async for c in self._paged(f"/repos/{self.repo}/issues/{number}/comments", conditional=True)] async def find_pr(self, branch: str) -> dict | None: owner = self.repo.split("/")[0] @@ -48,18 +48,19 @@ async def create_pr(self, title: str, body: str, head: str, base: str) -> dict: json={"title": title, "body": body, "head": head, "base": base})).json() async def get_pr(self, number: int) -> dict: - return (await self.request("GET", f"/repos/{self.repo}/pulls/{number}")).json() + return (await self.request("GET", f"/repos/{self.repo}/pulls/{number}", + conditional=True)).json() async def pr_reviews(self, number: int) -> list[dict]: - return [r async for r in self._paged(f"/repos/{self.repo}/pulls/{number}/reviews")] + return [r async for r in self._paged(f"/repos/{self.repo}/pulls/{number}/reviews", conditional=True)] async def pr_review_comments(self, number: int) -> list[dict]: - return [c async for c in self._paged(f"/repos/{self.repo}/pulls/{number}/comments")] + return [c async for c in self._paged(f"/repos/{self.repo}/pulls/{number}/comments", conditional=True)] async def check_state(self, sha: str) -> str: runs = (await self.request("GET", f"/repos/{self.repo}/commits/{sha}/check-runs", - params={"per_page": 100})).json()["check_runs"] - statuses = (await self.request("GET", f"/repos/{self.repo}/commits/{sha}/status")).json()["statuses"] + params={"per_page": 100}, conditional=True)).json()["check_runs"] + statuses = (await self.request("GET", f"/repos/{self.repo}/commits/{sha}/status", conditional=True)).json()["statuses"] return combine_checks(runs, statuses) async def merge_pr(self, number: int, sha: str) -> tuple[bool, str]: diff --git a/tests/test_autodev_activities.py b/tests/test_autodev_activities.py index e92e368..7869a10 100644 --- a/tests/test_autodev_activities.py +++ b/tests/test_autodev_activities.py @@ -272,3 +272,36 @@ async def test_poll_continues_after_one_issue_fails(tmp_path, gh): respx.get(f"{API}{R}/collaborators/alice/permission").mock(return_value=httpx.Response(200, json={"permission": "write"})) temporal = FakeTemporal() assert await acts(tmp_path, gh, temporal).poll_issues() == [2] + + +@respx.mock +async def test_observe_unchanged_pr_sends_etags_and_reuses_cache(tmp_path, gh): + head = "abc1234def" + bodies = { + f"{R}/pulls/5": {"state": "open", "merged": False, "head": {"sha": head}}, + f"{R}/pulls/5/reviews": [], f"{R}/pulls/5/comments": [], f"{R}/issues/5/comments": [], + f"{R}/commits/{head}/check-runs": {"check_runs": []}, f"{R}/commits/{head}/status": {"statuses": []}} + routes = {} + for path, body in bodies.items(): + routes[path] = respx.get(f"{API}{path}").mock(side_effect=[ + httpx.Response(200, json=body, headers={"etag": f'"{path}"'}), httpx.Response(304)]) + a = acts(tmp_path, gh) + first = await a.observe_pr(ObserveInput(5)) + second = await a.observe_pr(ObserveInput(5)) + assert second == first + for path, route in routes.items(): + assert "if-none-match" not in route.calls[0].request.headers + assert route.calls[1].request.headers["if-none-match"] == f'"{path}"' + + +@respx.mock +async def test_changed_resource_replaces_cached_etag(gh): + route = respx.get(f"{API}{R}/pulls/5").mock(side_effect=[ + httpx.Response(200, json={"n": 1}, headers={"etag": '"a"'}), + httpx.Response(200, json={"n": 2}, headers={"etag": '"b"'}), + httpx.Response(304)]) + assert (await gh.get_pr(5))["n"] == 1 + assert (await gh.get_pr(5))["n"] == 2 + assert (await gh.get_pr(5))["n"] == 2 + assert route.calls[1].request.headers["if-none-match"] == '"a"' + assert route.calls[2].request.headers["if-none-match"] == '"b"' diff --git a/zarr_doctor/github_api.py b/zarr_doctor/github_api.py index fa0cf51..fc6c024 100644 --- a/zarr_doctor/github_api.py +++ b/zarr_doctor/github_api.py @@ -40,6 +40,7 @@ def __init__(self, repo: str, token_fn, *, api_url: str = "https://api.github.co self._min_gap = 1.0 / writes_per_second self._creates_per_hour = creates_per_hour self._last_write = 0.0 + self._etags: dict[tuple, tuple[str, httpx.Response]] = {} # (path, params) -> (etag, last 200 response) self._creates: collections.deque[float] = collections.deque() # Contents API PUTs to one branch race (409 "is at X but expected Y"); serialize them in-process. self._commit_lock = asyncio.Lock() @@ -83,16 +84,21 @@ def _rate_limit_delay(r: httpx.Response, attempt: int) -> float | None: return None async def request(self, method: str, path: str, *, json: Any = None, params: dict | None = None, - ok404: bool = False, write: bool | None = None, attempts: int = 8) -> httpx.Response | None: + ok404: bool = False, write: bool | None = None, attempts: int = 8, + conditional: bool = False) -> httpx.Response | None: + """`conditional`: GET with If-None-Match; a 304 returns the response cached from the last 200 (free of rate limit).""" + ckey = (path, tuple(sorted((params or {}).items()))) if conditional and method == "GET" else None if write is None: write = method != "GET" create = write and method == "POST" for attempt in range(attempts): if write: await self._throttle(create) + headers = {"Authorization": f"Bearer {self._token_fn()}"} + if ckey in self._etags: + headers["If-None-Match"] = self._etags[ckey][0] try: - r = await self._client.request(method, path, json=json, params=params, - headers={"Authorization": f"Bearer {self._token_fn()}"}) + r = await self._client.request(method, path, json=json, params=params, headers=headers) except httpx.TransportError as e: log.warning("github %s %s transport error %s (attempt %d)", method, path, e, attempt + 1) await self._sleep(min(2**attempt, 60)) @@ -102,6 +108,8 @@ async def request(self, method: str, path: str, *, json: Any = None, params: dic log.warning("github rate limited on %s %s; sleeping %.0fs", method, path, delay) await self._sleep(delay) continue + if r.status_code == 304 and ckey in self._etags: + return self._etags[ckey][1] if r.status_code >= 500: await self._sleep(min(2**attempt, 60)) continue @@ -111,6 +119,11 @@ async def request(self, method: str, path: str, *, json: Any = None, params: dic raise GitHubError(f"{method} {path} -> {r.status_code}: {r.text[:500]}", r.status_code) if write: log.info("github %s %s -> %d", method, path, r.status_code) + if ckey is not None: + if r.headers.get("etag"): + self._etags[ckey] = (r.headers["etag"], r) + else: + self._etags.pop(ckey, None) return r raise GitHubError(f"{method} {path}: gave up after {attempts} attempts") @@ -129,10 +142,11 @@ async def graphql(self, query: str, variables: dict | None = None) -> dict: return body["data"] raise GitHubError("graphql: rate limited, gave up") - async def _paged(self, path: str, params: dict | None = None): + async def _paged(self, path: str, params: dict | None = None, conditional: bool = False): page = 1 while True: - items = (await self.request("GET", path, params={**(params or {}), "per_page": 100, "page": page})).json() + items = (await self.request("GET", path, params={**(params or {}), "per_page": 100, "page": page}, + conditional=conditional)).json() for it in items: yield it if len(items) < 100: