Skip to content

Commit 564d393

Browse files
thodson-usgsclaude
andcommitted
fix(utils): query() must not mutate the caller's payload
query() stringified each value in place (`payload[key] = to_str(...)`), so a caller reusing the same dict carried delimiter-joined values into its next call. Build a fresh `params` dict from the payload instead and pass that to the request; the caller's dict is left untouched. Adds a regression test asserting the payload is unchanged after the call. The branch's original 4xx/5xx-handling commits are dropped: that half is already superseded on main by the httpx migration (DOI-USGS#289) and the typed error taxonomy (`_raise_for_status` / `error_for_status`, DOI-USGS#313/DOI-USGS#319), which raise on every >=400 status. Only the payload-mutation fix remained unmerged, re-authored here onto main's httpx `query`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sjb14HkwuCydKSKMsaXsgd
1 parent 4daf771 commit 564d393

2 files changed

Lines changed: 17 additions & 5 deletions

File tree

dataretrieval/utils.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ def query(
419419
url: string
420420
URL to query
421421
payload: dict
422-
query parameters passed to ``httpx.get``
422+
query parameters passed to ``httpx.get``. Not mutated.
423423
delimiter: string
424424
delimiter to use with lists
425425
ssl_check: bool
@@ -442,18 +442,18 @@ def query(
442442
``httpx`` exception on ``__cause__``.
443443
"""
444444

445-
for key, value in payload.items():
446-
payload[key] = to_str(value, delimiter)
445+
# Build a fresh params dict; never mutate the caller's payload.
446+
params = {key: to_str(value, delimiter) for key, value in payload.items()}
447447
# httpx serializes None params as ``foo=``; USGS rejects with 400.
448448
# Drop them. (``to_str`` returns None for non-iterable scalars like bools.)
449-
payload = {k: v for k, v in payload.items() if v is not None}
449+
params = {k: v for k, v in params.items() if v is not None}
450450

451451
user_agent = {"user-agent": f"python-dataretrieval/{dataretrieval.__version__}"}
452452

453453
try:
454454
response = _get(
455455
url,
456-
params=payload,
456+
params=params,
457457
headers=user_agent,
458458
verify=ssl_check,
459459
**HTTPX_DEFAULTS,

tests/utils_test.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Unit tests for functions in utils.py"""
22

3+
import copy
34
from unittest import mock
45

56
import pandas as pd
@@ -33,6 +34,17 @@ def test_header(self, httpx_mock):
3334
assert response.status_code == 200 # GET was successful
3435
assert "user-agent" in response.request.headers
3536

37+
def test_does_not_mutate_caller_payload(self, httpx_mock):
38+
"""query() builds its own params dict and must not mutate the caller's
39+
payload, which it previously stringified in place (so a reused dict
40+
carried delimiter-joined values into the next call)."""
41+
httpx_mock.add_response(method="GET", json={"value": {}})
42+
url = "https://waterservices.usgs.gov/nwis/dv"
43+
payload = {"sites": ["01646500", "01646501"], "format": "json"}
44+
original = copy.deepcopy(payload)
45+
utils.query(url, payload)
46+
assert payload == original
47+
3648

3749
class Test_error_taxonomy:
3850
"""The unified request-error hierarchy.

0 commit comments

Comments
 (0)