Skip to content

refactor(ogc): close over request state, retiring ogc.context - #367

Merged
thodson-usgs merged 1 commit into
DOI-USGS:mainfrom
thodson-usgs:refactor/ogc-close-over-request-state
Aug 11, 2026
Merged

refactor(ogc): close over request state, retiring ogc.context#367
thodson-usgs merged 1 commit into
DOI-USGS:mainfrom
thodson-usgs:refactor/ogc-close-over-request-state

Conversation

@thodson-usgs

Copy link
Copy Markdown
Collaborator

What

Retires the ambient trio — ogc/context.py's base-URL, dialect, and row-cap ContextVars — in favor of explicit binding, and drops the copy_context() snapshot from FanOut.

Before this change the same three values traveled by three mechanisms at once: partial-bound finalizer arguments, ambient ContextVars, and .get() fallbacks deep inside request construction and shaping. No signature stated they were required, and the service-neutral FanOut executor snapshotted an entire execution context at construction (with a ten-line comment explaining which adapter ambients it must not drop) solely so a resume fired after the originating with-blocks exited would rebuild chunks against the right API.

How

  • _construct_api_requests / _construct_cql_request take base_url (and dialect) as explicit parameters. get_ogc_data binds them with functools.partial — the pattern its finalizer already used — into both the plan's build_request and the per-chunk fetch, so planning-time sizing and a later exc.call.resume() rebuild every chunk against the values the call was created with.
  • row_cap is a real parameter of _paginate/_walk_pages, forwarded to the paginate parameter transport already had, instead of an ambient read inside a compatibility wrapper.
  • _finalize_ogc / _deal_with_empty require base_url; the empty-frame schema lookup can no longer silently target an ambient default.
  • FanOut loses self._ctx and _resume_in_context: transport carries no adapter state (ADR 0006), and the progress reporter reaches the worker thread through the portal's normal calling-context copy.
  • waterdata.get_cql states its target explicitly instead of importing a private ambient and entering its scope.
  • ogc/context.py is deleted; ADR 0002's prose and the architecture overview are updated to match.

Why

Interfaces now state their preconditions (the base_url-omitted footgun documented on get_ogc_data is caught by the signature everywhere, not just at the one required call site), the resume-after-return invariant is structural rather than guarded by a snapshot and prose, and the one adapter-shaped concern inside service-neutral transport is gone.

Testing

  • Full offline suite: 780 passed.
  • mypy --strict, ruff check/format, xenon, complexipy, and lint-imports (7 contracts) all pass.
  • The captured-context regression test is rewritten as a creation-time-binding test; the direct request-construction unit tests bind the Water Data target with the same partial pattern the engine uses.

🤖 Generated with Claude Code

The base URL, dialect, and row cap traveled by three mechanisms at
once: partial-bound finalizer arguments, ambient ContextVars in
ogc.context, and .get() fallbacks deep in request construction and
shaping. No interface stated that they were required, and the
service-neutral FanOut executor snapshotted a whole execution context
at construction solely to keep that adapter state alive across a
resume fired after the originating with-blocks had exited.

Bind them the way the finalizer was already bound instead:

- _construct_api_requests / _construct_cql_request take base_url (and
  dialect) as explicit parameters; get_ogc_data binds them with
  functools.partial into both the plan's build_request and the fetch,
  so planning-time sizing and a later exc.call.resume() rebuild every
  chunk against the values the call was created with.
- row_cap is a real parameter of _paginate/_walk_pages, forwarded to
  the transport paginate parameter that already existed, instead of
  being laundered through an ambient read inside a wrapper.
- _finalize_ogc / _deal_with_empty require base_url; the empty-frame
  schema lookup can no longer silently target the ambient default.
- FanOut drops the copy_context() snapshot and _resume_in_context:
  transport carries no adapter state (ADR 0006), and the progress
  reporter reaches the worker through the portal's normal
  calling-context copy.
- waterdata.get_cql states its target explicitly instead of entering
  a private ambient scope.
- ogc/context.py is deleted.

The captured-context regression test becomes a creation-time-binding
test; the request-construction unit tests bind the Water Data target
with the same partial pattern the engine uses.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@thodson-usgs

Copy link
Copy Markdown
Collaborator Author

Health metrics vs main

Measured locally with the same tooling CI runs (pyscn 1.29.0, radon, xenon, complexipy, import-linter), each branch in its own worktree so import resolution is comparable — ProjectRoot verified to equal the checkout in every run, per the sanity check in code-health.yml.

pyscn composite: 82/100 (B) — unchanged, and all seven sub-scores are identical to main (Complexity 95, Dead code 100, Duplication 70, CBO 100, LCOM 100, Dependencies 80, Architecture 87).

Underneath the banded scores:

main this PR
Modules 57 56
Resolved dependency edges 140 135
Max dependency depth 8 8
Technical-debt estimate 162 h 160 h
Functions 363 361
High-risk entries 19 19
Package LOC (non-blank) 13,090 13,079

Per-function movement (radon cyclomatic):

  • _finalize_ogc 8 → 7, _deal_with_empty 4 → 3 — the ambient .get() fallbacks were branches.
  • _construct_api_requests 5 → 6 — the dialect is None default is a new branch; that is the cost of making the parameter explicit.
  • Removed: FanOut._resume_in_context, ogc/context.py.

Maintainability index (radon): ogc/requests.py 48.3 → 49.6, ogc/policy.py 56.1 → 57.2, ogc/engine.py 61.7 → 61.9, waterdata/cql.py 83.4 → 84.1; ogc/shaping.py 45.1 → 44.8 and transport/fanout.py 27.7 → 27.5 are the two small regressions. All stay rank A.

One number that looks like a regression but isn't: duplication reads 8.83% → 8.90%. Clone groups (5), clone pairs (50), and total clones (25) are identical — no new duplication. The package got smaller, so the same clones are a larger fraction of it.

Merge gates (xenon --max-absolute C --max-modules B --max-average A, complexipy, lint-imports, mypy --strict): pass, as on main.

@thodson-usgs
thodson-usgs marked this pull request as ready for review August 11, 2026 12:52
@thodson-usgs
thodson-usgs merged commit f31b473 into DOI-USGS:main Aug 11, 2026
11 checks passed
@thodson-usgs
thodson-usgs deleted the refactor/ogc-close-over-request-state branch August 11, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant