refactor(transport): one driver for paginate-through-the-executor - #371
Merged
thodson-usgs merged 1 commit intoAug 12, 2026
Merged
Conversation
thodson-usgs
force-pushed
the
refactor/run-paginated-driver
branch
from
August 12, 2026 21:29
27f4243 to
c5c0a63
Compare
Three adapters carried the same ~20-line scaffold -- build a fetch that hands each request to paginate, borrow the executor's shared client, construct FanOut with RetryPolicy.from_env() and a first-request canonical URL, resume -- and had already drifted: only Statistics honored a caller-injected client, only Water Use mapped raw transport errors, and each spelled the canonical-URL convention itself. run_paginated (transport/pagination.py) is that scaffold's one home. An adapter now supplies only its strategies -- what a page is, where the cursor lives, how errors decode, and optionally how to shape the result -- exactly the division of labor wateruse._fan_out's docstring already claimed for itself; now it is true in code for all three: - stats.get_data: one-item plan, caller-injected client honored via the driver's client= parameter. - wateruse._fan_out: per-location plan; its TransportError -> network_error wrap is deleted rather than ported, because the executor's _normalize_failure already produces the identical typed NetworkError -- test_permanent_transport_failure_remains_a_network_error passes unchanged against the driver (patch retargeted to the page walk's home). - ratings._search: one-item plan, verify= via client_options. No layers-contract change: transport is one layer, and the new pagination -> fanout edge cannot cycle (fanout does not import pagination; adapters composed the two, which is exactly what moved down here). Full offline suite: 786 passed. mypy --strict, ruff, xenon, complexipy, import-linter all pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BTaSm7HmVb94RSJiKW4WAS
thodson-usgs
force-pushed
the
refactor/run-paginated-driver
branch
from
August 12, 2026 21:38
c5c0a63 to
5f58b1b
Compare
thodson-usgs
marked this pull request as ready for review
August 12, 2026 21:40
thodson-usgs
added a commit
to thodson-usgs/dataretrieval-python
that referenced
this pull request
Aug 12, 2026
Absorbs DOI-USGS#368, DOI-USGS#369, and DOI-USGS#371. Eight files conflicted; three of the resolutions were more than textual: - ``wateruse.py``: main edited the implementation this branch had already renamed to ``nwdc.py``, so git conflicted the shim against it. Kept the shim and ported DOI-USGS#371's ``run_paginated`` migration into ``nwdc.py``, preserving ``adapter="nwdc"``. - ``cql.py``: took main's version wholesale. This branch's two changes there (``redirected(OGC_API_URL)`` and ``adapter="waterdata"``) are subsumed by DOI-USGS#368 routing ``get_cql`` through ``waterdata.utils.get_ogc_data``, which already applies both. - ``ratings.py``: took main's rewritten implementation and re-applied this branch's only contribution to it (``redirected(STAC_URL)``), plus adapter scoping on both drives. Three breaks were semantic, not textual -- git merged them cleanly and they would have failed at import or call time, because main added callers of names this branch renamed or deleted: - ``transport/pagination.run_paginated`` imported ``_CONCURRENCY_DEFAULT`` (deleted here in favour of ``configuration.DEFAULT_CONCURRENCY``) and called ``RetryPolicy.from_env`` (renamed to ``from_configuration``). It now takes an ``adapter`` argument and threads it to both the retry policy and the executor, which is what makes per-adapter ``retries`` and ``concurrency`` tables reach the three getters that use it. - ``ogc/engine``'s ``cql_body`` branch (new in DOI-USGS#368) called ``from_env`` and dropped the adapter; both fixed. - ``get_ogc_data`` gained ``cql_body`` from main and ``adapter`` here; both parameters kept. 969 passed, mypy --strict clean, all hooks including import-linter pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BTaSm7HmVb94RSJiKW4WAS
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
1 participant
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.
What
Extracts
run_paginatedintotransport/pagination.py: the one home for the "compose paginate strategies into a fetch, hand the plan toFanOut, resume" scaffold, and migrates its three copies —stats.get_data,wateruse._fan_out, andratings._search— onto it.Why
The scaffold existed three times (#369 added the third) and had already drifted: only Statistics honored a caller-injected client, only Water Use mapped raw transport errors to the typed taxonomy, and each spelled the first-request canonical-URL convention itself. An adapter should supply only its strategies — what a page is, where the cursor lives, how errors decode, how to shape the result — which is the division of labor
wateruse._fan_out's own docstring already claimed. Now it is true in code for all three.Notable call
Water Use's in-fetch
TransportError → network_errorwrap is deleted rather than ported. The executor's_normalize_failurealready produces the identical typedNetworkErrorfor deterministic transport failures, so the wrap was redundant with machinery that landed after it was written.test_permanent_transport_failure_remains_a_network_errorpins this and passes unchanged against the driver (its monkeypatch retargeted to the page walk's home).No layers-contract change:
transportis a single layer, and the newpagination → fanoutedge cannot cycle (fanoutdoes not importpagination; the adapters composed the two, which is precisely what moved down).Testing
mypy --strict,ruff,xenon,complexipy,lint-importsall pass.Rebased onto main after #369 merged; the diff is this PR's single commit.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BTaSm7HmVb94RSJiKW4WAS