Apply review feedback to PR DOI-USGS#277 + bump Actions to Node-24

Simplify-review consensus across three reviewers (reuse / quality /
efficiency) on the conftest.py from the previous commit:

- Drop the `pytest.mark.live` marker double-application: no `-m live`
  selector is wired anywhere in CI; only `pytest.mark.flaky` carries
  behavior. The `live` marker plus its `pytest_configure` registration
  was dead metadata.
- Tighten `_TRANSIENT_PATTERNS`: drop `r"timed out"` (subsumed by the
  `Timeout` arm) and `r"Bad Gateway|Service Unavailable|Gateway Timeout"`
  (subsumed by the `RuntimeError:\s*5\d\d:` prefix from
  `_raise_for_non_200`). Three load-bearing patterns left.
- Drop defensive `getattr(item, "fixturenames", ())` — pytest items
  reliably expose `fixturenames`; the default-tuple fallback is dead
  defense.
- Shorten the module docstring: keep the WHY paragraph (PR DOI-USGS#273 →
  surfaced errors → CI flake risk) and drop the WHAT bullets that
  narrate the code below.
- Add a one-line justification for the 5-second backoff (USGS
  upstream typically recovers from brief 5xx within that window).
- Tighten the NEWS.md entry: drop conftest implementation detail
  (registers a marker, applies via collection hook, …) and keep the
  user-relevant claim.

Skipped from the review (reviewer can decide):
- Inverting the auto-mark heuristic to opt-in `@pytest.mark.live`
  would force decorating ~35 test functions; the current
  "everything not using requests_mock is live" is zero-cost on
  success and self-extending for new tests.
- Centralizing the transient-error regex set in `dataretrieval/
  waterdata/utils.py` would couple library code to test
  infrastructure for one pattern.

Separately, bump GitHub Actions to Node-24-compatible versions
(@v6 across all three workflows) ahead of the deprecation
warning surfaced on the previous CI run: actions/checkout@v4 → v6,
actions/setup-python@v5 → v6. Node 20 is deprecated and will be
removed from runners on 2026-09-16; the default flips on
2026-06-02. NEWS.md entry updated to mention the bump.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: thodson-usgs

.github/workflows/python-package.yml

@@ -13,9 +13,9 @@ jobs:
   lint:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v6
       - name: Set up Python 3.14
-        uses: actions/setup-python@v5
+        uses: actions/setup-python@v6
         with:
           python-version: "3.14"
           cache: "pip"
@@ -36,9 +36,9 @@ jobs:
         python-version: ["3.9", "3.13", "3.14"]
 
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v6
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v5
+        uses: actions/setup-python@v6
         with:
           python-version: ${{ matrix.python-version }}
           cache: "pip"

.github/workflows/python-publish.yml

@@ -21,9 +21,9 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v6
     - name: Set up Python
-      uses: actions/setup-python@v5
+      uses: actions/setup-python@v6
       with:
         python-version: '3.x'
         cache: 'pip'

.github/workflows/sphinx-docs.yml

@@ -11,11 +11,11 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v6
         with:
           persist-credentials: false
       - name: Set up Python
-        uses: actions/setup-python@v5
+        uses: actions/setup-python@v6
         with:
           python-version: "3.13"
           cache: "pip"

NEWS.md

@@ -1,4 +1,4 @@
-**05/15/2026:** Live-API tests are now auto-retried on transient upstream errors. PR #273 (above) intentionally surfaces mid-pagination HTTP failures to callers instead of silently truncating results, which made any CI test that hits the live USGS Water Data API susceptible to flaking on transient 429/5xx/connection blips — a 502 Bad Gateway from upstream broke CI on the #273 merge to main. A new `tests/conftest.py` registers a `live` pytest marker, auto-applies it to every test that does not take `requests_mock` (the existing mock-driven convention), and via `pytest-rerunfailures` retries those tests up to twice on a 5-second backoff — but only when the failure trace matches a narrow transient-upstream pattern (`429:` / `5xx:` prefixes from `_raise_for_non_200`, plus `ConnectionError` / timeout shapes). Library bugs raising other exception types still fail on the first attempt.
+**05/15/2026:** CI test suite is now resilient to transient USGS Water Data API blips. PR #273 (above) intentionally surfaces mid-pagination HTTP errors to callers rather than silently truncating — the right behavior, but it made any live-API test in CI flaky against transient 429/5xx/connection errors. A new `tests/conftest.py` uses `pytest-rerunfailures` to retry live-API tests up to twice on a 5-second backoff, but only when the failure trace matches a narrow transient-upstream pattern. Library bugs raising other exception types still fail on the first attempt. Also bumps `actions/checkout` and `actions/setup-python` to Node-24-compatible versions (`@v6`) ahead of the GitHub Actions Node 20 runtime deprecation.
 
 **05/14/2026:** Fixed two latent bugs in the paginated `waterdata` request loop (`_walk_pages` and `get_stats_data`). Previously, when `requests.Session.request(...)` itself raised mid-pagination (network error, timeout), the except block called `_error_body()` on the *prior page's* response, so the logged "error" described the wrong request and could itself crash on non-JSON bodies. Separately, no status-code check was performed on subsequent paginated responses, so a 5xx body that didn't include `numberReturned` was silently treated as an empty page — pagination quietly stopped and the user got truncated data with no error logged. The loop now status-checks each page like the initial request and reports the actual exception. The "best-effort" behavior (return whatever pages were collected) is preserved.
 

tests/conftest.py

@@ -1,71 +1,41 @@
-"""Test infrastructure shared across all test modules.
-
-After #273 the paginated ``waterdata`` getters surface mid-walk HTTP
-errors (429 / 5xx / connection drops) to the caller instead of silently
-truncating the result. That's the correct behavior for users — but it
-makes any CI test that hits the live USGS Water Data API susceptible to
-flaking on a transient upstream blip (e.g. the 502 Bad Gateway that
-broke CI on #273's merge to main).
-
-This file:
-
-* registers a ``live`` pytest marker;
-* auto-applies it to every test that does **not** take ``requests_mock``
-  as a fixture — the existing convention in this repo for mock-driven
-  tests, so the marker tracks "this test hits the network" without
-  needing to decorate 35 functions by hand;
-* via ``pytest-rerunfailures``, configures live-marked tests to retry
-  up to twice (5-second backoff) ONLY when the failure trace matches a
-  transient-upstream pattern: ``429:`` / ``5xx:`` prefixes that
-  ``_raise_for_non_200`` produces, plus ``ConnectionError`` / timeout
-  shapes from the ``requests`` library.
-
-Library bugs that raise unrelated exception types are NOT retried —
-the regex set deliberately omits generic ``RuntimeError`` matches.
+"""Auto-retry live-API tests on transient upstream errors.
+
+After PR #273, paginated ``waterdata`` getters propagate mid-walk HTTP
+errors (429 / 5xx / connection drops) instead of silently truncating the
+result. That's the correct behavior for users but makes any CI test that
+hits the live USGS Water Data API susceptible to flaking on a transient
+upstream blip — e.g. the HTTP 502 Bad Gateway that broke CI on PR #273's
+merge to main.
+
+Heuristic: any test that does NOT request the ``requests_mock`` fixture
+is treated as live and gets retried on transient-error patterns only.
+Library bugs raising other exception types still fail on the first try.
 """
 
 import pytest
 
-# Match anywhere in the failure traceback. Anchor-free because the
-# trace embeds the exception message after a long preamble.
+# Anchored loosely — the failure trace embeds the exception message
+# after a long preamble.
 _TRANSIENT_PATTERNS = [
     r"RuntimeError:\s*(?:429|5\d\d):",  # _raise_for_non_200 output
     r"ConnectionError",  # requests/urllib3
     r"ReadTimeout|ConnectTimeout|Timeout",
-    r"timed out",
-    r"Bad Gateway|Service Unavailable|Gateway Timeout",
 ]
 
+# 5 seconds is generous enough for a USGS upstream replica to recover
+# from a brief 5xx but short enough that CI doesn't drag.
+_RETRY_DELAY_SEC = 5
 _MAX_RERUNS = 2
-_RERUN_DELAY_SEC = 5
-
-
-def pytest_configure(config):
-    """Register the ``live`` marker so pytest doesn't warn about it."""
-    config.addinivalue_line(
-        "markers",
-        "live: marks tests that hit the live USGS Water Data API; "
-        "retried up to twice on transient upstream HTTP errors.",
-    )
 
 
 def pytest_collection_modifyitems(config, items):
-    """Auto-mark live-API tests with ``live`` + ``flaky`` retry config.
-
-    Heuristic: any test that does NOT request the ``requests_mock``
-    fixture is treated as live. This catches every test in
-    ``waterdata_test.py`` and similar modules without needing to
-    decorate them individually, and it auto-tracks new tests written
-    in the same style.
-    """
     for item in items:
-        if "requests_mock" in getattr(item, "fixturenames", ()):
+        if "requests_mock" in item.fixturenames:
             continue
-        item.add_marker(pytest.mark.live)
         item.add_marker(
             pytest.mark.flaky(
                 reruns=_MAX_RERUNS,
-                reruns_delay=_RERUN_DELAY_SEC,
+                reruns_delay=_RETRY_DELAY_SEC,
                 only_rerun=_TRANSIENT_PATTERNS,
             )
         )