Skip to content

Commit 200262a

Browse files
thodson-usgsclaude
andcommitted
Address Copilot PR #273 review: include next_token in pagination warning
Copilot noted that the get_stats_data pagination-failure warning logs only the base statistics URL (constant across all pages), dropping the next_token cursor that uniquely identifies which page in the sequence failed. The original `resp.url` reference was abandoned because `resp` may be stale when client.request() itself raises — but next_token is the loop variable and is always current. Add `(next_token=<value>)` to the warning, plus a focused test that asserts the cursor surfaces in the WARNING-level log entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 44c434b commit 200262a

2 files changed

Lines changed: 24 additions & 1 deletion

File tree

dataretrieval/waterdata/utils.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1149,7 +1149,10 @@ def get_stats_data(
11491149
except Exception as e: # noqa: BLE001
11501150
logger.exception("Request incomplete: %s", e)
11511151
logger.warning(
1152-
"Request failed for URL: %s. Data download interrupted.", url
1152+
"Request failed for URL: %s (next_token=%s). "
1153+
"Data download interrupted.",
1154+
url,
1155+
next_token,
11531156
)
11541157
next_token = None
11551158

tests/waterdata_utils_test.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,26 @@ def test_get_stats_data_surfaces_5xx_mid_pagination(caplog, monkeypatch):
236236
assert any("503" in m or "ServiceUnavailable" in m for m in messages), messages
237237

238238

239+
def test_get_stats_data_warning_includes_next_token(caplog, monkeypatch):
240+
"""The pagination-failure warning includes the next_token so operators
241+
can identify which page in the sequence failed. (Addresses Copilot's
242+
PR #273 review note: the base URL alone drops cursor context.)"""
243+
caplog.set_level(logging.WARNING, logger=_LOGGER_NAME)
244+
245+
page2_503 = mock.MagicMock()
246+
page2_503.status_code = 503
247+
page2_503.json.return_value = {
248+
"code": "ServiceUnavailable",
249+
"description": "upstream timeout",
250+
}
251+
252+
_run_get_stats_data_with_failure(page2_503, monkeypatch)
253+
254+
warnings_ = [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING]
255+
# The initial response from _stats_initial_ok carries next=tok2.
256+
assert any("tok2" in m for m in warnings_), warnings_
257+
258+
239259
def test_handle_stats_nesting_tolerates_missing_drop_columns():
240260
"""If the upstream stats response shape ever changes such that one of
241261
the columns we try to drop ("type", "properties.data") is absent, the

0 commit comments

Comments
 (0)