Skip to content

Commit ad1208e

Browse files
thodson-usgsclaude
andcommitted
docs(waterdata): editorial pass for readability on chunking.py
Tighten seven prose-heavy passages where the wording made the reader back up to re-parse. No semantic changes. - RetryPolicy.__post_init__ comment: lead with intent ('catch invalid knobs at construction'); keep the why-not (asyncio.sleep silent vs time.sleep loud) as a clarifying parenthetical. - RetryPolicy.from_env docstring: split the comma-and-semicolon chain into one cleaner sentence; lead with the verb ('Reads...'). - _chunked_client comment: drop the 'across every gathered sub-request of the call' tail and the 'in that case' coda. - _set_response_url docstring: lead with the control flow (try direct first; on real responses, swap the bound request) rather than the read-only-vs-writable mechanism. - _retry_delay docstring: drop the stale 'sync and async drivers share it' line left over from before the async-only collapse; format the three None cases as an em-dash list. - ChunkedCall class docstring: split the long opener at the natural sentence boundary instead of trailing it with 'and ... and ... — used both for...'. - _pending docstring: replace the awkward 'The single source of the "walk ..., skip ..." rule' construction with a direct two-sentence description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 01c734b commit ad1208e

1 file changed

Lines changed: 32 additions & 35 deletions

File tree

dataretrieval/waterdata/chunking.py

Lines changed: 32 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -222,10 +222,10 @@ class RetryPolicy:
222222
retry_after_cap: float = _RETRY_AFTER_CAP
223223

224224
def __post_init__(self) -> None:
225-
# Guard the value object's own invariants so a misconfiguration
226-
# fails loudly at construction rather than as a downstream
227-
# ``time.sleep`` ValueError (negative delay) or a silent
228-
# asyncio.sleep-treats-negative-as-zero divergence.
225+
# Catch invalid timing knobs here so a misconfiguration fails at
226+
# construction, not deep in a later ``time.sleep`` (ValueError on
227+
# a negative delay) or silently in ``asyncio.sleep`` (which
228+
# treats negative as zero).
229229
if self.max_retries < 0:
230230
raise ValueError(f"max_retries must be >= 0 (got {self.max_retries}).")
231231
if self.base_backoff < 0 or self.max_backoff < 0 or self.retry_after_cap < 0:
@@ -236,10 +236,10 @@ def from_env(cls) -> RetryPolicy:
236236
"""
237237
Build a policy from the module-level defaults, resolved now.
238238
239-
``max_retries`` comes from ``API_USGS_RETRIES``; the timing knobs
240-
are read from the ``_RETRY_*`` module constants at call time (not
241-
the dataclass field defaults, which freeze at class definition) so
242-
``monkeypatch.setattr`` on those constants takes effect.
239+
Reads ``max_retries`` from ``API_USGS_RETRIES`` and the timing
240+
knobs from the ``_RETRY_*`` module constants at call time not
241+
the dataclass field defaults (which freeze at class definition)
242+
— so test ``monkeypatch.setattr`` on the constants takes effect.
243243
244244
Returns
245245
-------
@@ -308,12 +308,11 @@ def backoff(self, attempt: int, retry_after: float | None) -> float:
308308
_NO_RETRY = RetryPolicy(max_retries=0)
309309

310310

311-
# The single shared ``httpx.AsyncClient`` of an in-flight chunked call,
312-
# published (via :func:`_publish`) during ``ChunkedCall._run`` so the
313-
# paginated-loop helpers downstream (``_walk_pages``) reuse one
314-
# connection pool across every gathered sub-request of the call. ``None``
315-
# when not inside a chunked call — paginated helpers fall back to their
316-
# own short-lived client in that case.
311+
# Shared per-call ``httpx.AsyncClient``, published via :func:`_publish`
312+
# during ``ChunkedCall._run`` so paginated-loop helpers (``_walk_pages``)
313+
# reuse the same connection pool across every sub-request. ``None``
314+
# outside a chunked call — paginated helpers then open their own
315+
# short-lived client.
317316
_chunked_client: ContextVar[httpx.AsyncClient | None] = ContextVar(
318317
"_chunked_client", default=None
319318
)
@@ -676,13 +675,12 @@ def _set_response_url(response: httpx.Response, url: str | httpx.URL) -> None:
676675
Overwrite the URL surfaced by a response without back-propagating
677676
the change into any aliased original.
678677
679-
On real ``httpx.Response`` instances ``.url`` is a read-only
680-
property that resolves through the bound request; rather than
681-
mutate the existing request's URL (which would be visible through
682-
any shallow copy that shares the same ``.request``), we replace
683-
the response's request with a fresh :class:`httpx.Request` carrying
684-
the new URL. On lightweight test mocks ``.url`` is a plain
685-
writable attribute — that path is tried first.
678+
Try the direct assignment first: on lightweight test mocks ``.url``
679+
is a plain writable attribute. On real ``httpx.Response`` it's
680+
read-only (it resolves through the bound request), so swap in a
681+
fresh :class:`httpx.Request` carrying the new URL — mutating the
682+
existing one would leak through any shallow copy that shares the
683+
same ``.request``.
686684
"""
687685
try:
688686
response.url = url # type: ignore[misc]
@@ -1141,12 +1139,11 @@ def _retry_delay(exc: BaseException, attempt: int, policy: RetryPolicy) -> float
11411139
Decide the backoff for a just-failed ``attempt`` (1-based), or ``None``
11421140
to give up and re-raise.
11431141
1144-
Returns ``None`` when the error isn't a retryable transient, the policy
1145-
is exhausted, or the server's ``Retry-After`` is too long to absorb
1146-
inline (so it escalates to a resumable :class:`ChunkInterrupted`).
1147-
Otherwise returns the seconds to wait and emits the progress-bar retry
1148-
note. This is the whole retry *decision* — the sync and async drivers
1149-
share it and differ only in how they call the fetch and how they sleep.
1142+
Returns ``None`` in three cases — the error isn't a retryable
1143+
transient, the policy is exhausted, or the server's ``Retry-After``
1144+
exceeds the cap (escalates to a resumable :class:`ChunkInterrupted`
1145+
instead). Otherwise returns the seconds to wait and emits the
1146+
progress-bar retry note.
11501147
11511148
Parameters
11521149
----------
@@ -1339,10 +1336,10 @@ class ChunkedCall:
13391336
Stateful handle for a chunked call.
13401337
13411338
Holds the in-flight state (per-sub-request frames and responses)
1342-
and the async fetcher, and exposes a single :meth:`resume` entry
1343-
point that drives the call from wherever it is to completion — used
1344-
both for the first invocation (from :meth:`ChunkPlan.execute`) and
1345-
for subsequent retries after a :class:`ChunkInterrupted`.
1339+
and the async fetcher. A single :meth:`resume` entry point drives
1340+
the call from wherever it is to completion — used both for the
1341+
first invocation (from :meth:`ChunkPlan.execute`) and for subsequent
1342+
retries after a :class:`ChunkInterrupted`.
13461343
13471344
:meth:`_run` gathers every pending sub-request over one shared
13481345
:class:`httpx.AsyncClient`, applies the failure-precedence rules, and
@@ -1553,10 +1550,10 @@ def _pending(self) -> Iterator[tuple[int, dict[str, Any]]]:
15531550
"""
15541551
Yield ``(index, sub_args)`` for sub-requests not yet completed.
15551552
1556-
The single source of the "walk :meth:`ChunkPlan.iter_sub_args` in
1557-
deterministic order, skip any index already in ``self._chunks``"
1558-
rule that :meth:`_run` uses to decide *which* sub-requests it
1559-
still owes (first run and every resume alike).
1553+
Walks :meth:`ChunkPlan.iter_sub_args` in deterministic order
1554+
and skips any index already in ``self._chunks``. :meth:`_run`
1555+
uses this to pick up exactly the sub-requests it still owes —
1556+
first run and every resume alike.
15601557
15611558
Yields
15621559
------

0 commit comments

Comments
 (0)