Skip to content

Add generic bounded-retry helper for source-adapter network IO - #6

Merged
mengerj merged 3 commits into
mainfrom
add-generic-retry-helper
Jun 24, 2026
Merged

Add generic bounded-retry helper for source-adapter network IO#6
mengerj merged 3 commits into
mainfrom
add-generic-retry-helper

Conversation

@mengerj

@mengerj mengerj commented Jun 24, 2026

Copy link
Copy Markdown
Owner

What

Adds a small, generic (non-Azure) bounded-retry-with-jittered-backoff helper and wires it into the source adapters' network IO.

  • New src/parce/sources/_retry.pywith_retries(func, ...) + is_transient(exc). Exponential backoff with full jitter, dependency-neutral (no Azure, no Census coupling). Lives in sources/ so it's mypy-checked stable core and reusable by the GEO (PR 5) and PRIDE (PR 7) adapters.
  • sources/cellxgene.py — wraps open_soma, the datasets table read, get_obs, and get_source_h5ad_uri.
  • sources/publication.py — wraps requests.get and raise_for_status() together inside the retried call, so a 429/5xx is retried rather than raised straight out.
  • CLAUDE.md — updated the stale resilience reference (it pointed at "the existing helpers in main.py") to the new sources/_retry.py.
  • Teststests/test_retry.py (transient classification + retry/exhaustion, time.sleep patched) and tests/test_publication.py (offline 503 / connection-error → success wiring).

Why

PR 3 removed the Azure-keyed _is_transient/_backoff_delay helpers from main.py along with the LLM call (their only caller), leaving the CELLxGENE/EuropePMC network IO with no retry/backoff. PR 3's own session log flagged this as a follow-up; this PR closes it. It's an out-of-band resilience fix, not a roadmap PR — Next up stays PR 4.

Reviewer notes

  • Transient detection covers stdlib TimeoutError/ConnectionError/OSError, requests connection/timeout errors, and requests HTTPError with 429/5xx.
  • Gotcha worth a look: every requests exception subclasses OSError, so a naive "OSError → transient" check would wrongly retry a 404 or a malformed-URL error. is_transient therefore classifies requests exceptions first (by kind / status code) and only falls back to the stdlib check for non-requests errors. This is covered by tests (HTTPError(404) and MissingSchema/InvalidURL assert non-transient).
  • This branch was authored before PR 3 merged, then merged on top of it. The two source-file edits auto-merged onto PR 3's renamed adapter files; conflicts in sources/__init__.py (took PR 3's) and docs/ROADMAP.md (kept both session-log entries) were resolved by hand.

Gates

All four pass locally, including the hermetic run with .env moved aside:
ruff check · ruff format --check · mypy src/parce (17 files) · pytest -m "not integration"71 passed.

🤖 Generated with Claude Code

mengerj and others added 3 commits June 24, 2026 11:34
Add a non-Azure with_retries/is_transient helper in
src/parce/sources/_retry.py (exponential backoff, full jitter) and wrap the
CELLxGENE Census and EuropePMC network IO with it. Transient detection covers
stdlib timeout/connection/OS errors plus requests connection/timeout errors and
HTTP 429/5xx; requests exceptions are classified first since they subclass
OSError. Update the stale CLAUDE.md resilience reference and add offline tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts:
#	docs/ROADMAP.md
#	src/parce/sources/__init__.py
The post-merge rename left the test importing/patching the deleted
parce.tools.ncbi_fetcher module; point it at the new sources.publication path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mengerj
mengerj merged commit 64ed4f4 into main Jun 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant