Skip to content

feat(config)!: resolve settings through a layered chain - #353

Draft
thodson-usgs wants to merge 20 commits into
DOI-USGS:mainfrom
thodson-usgs:worktree-config-fallback-352
Draft

feat(config)!: resolve settings through a layered chain#353
thodson-usgs wants to merge 20 commits into
DOI-USGS:mainfrom
thodson-usgs:worktree-config-fallback-352

Conversation

@thodson-usgs

@thodson-usgs thodson-usgs commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Closes #352.

Adds dataretrieval.configuration: one ordered chain that resolves every setting, and a
configure() block that scopes settings to a single call, a single thread, or a single
service — without mutating process-global os.environ.

The shape

A configuration profile is a named set of settings for one adapter, written in
code or stored in the config file. configure() takes those objects positionally, at
most one per adapter, and nothing else:

import dataretrieval
from dataretrieval import ngwmn, waterdata, wqp
from dataretrieval.ngwmn import NgwmnConfiguration
from dataretrieval.waterdata import WaterdataConfiguration
from dataretrieval.wqp import WqpConfiguration

with dataretrieval.configure(
    WaterdataConfiguration.load("overnight"),  # from the file, by name
    NgwmnConfiguration.load("gentle"),         # from the file, by name
    WqpConfiguration(retries=2),               # built here
):
    flow, _ = waterdata.get_daily(monitoring_location_id=sites, time="P30D")
    levels, _ = ngwmn.get_water_level(monitoring_location_id=wells)
    samples, _ = wqp.get_results(siteid=sites)

That block is the case the feature exists for: several services, one configuration each,
some loaded from the file by name and some built in code. The adapter a configuration
targets is a property of its class, so the caller never restates it — which is what
keeps the adapter roster out of every call site. Configuration targets none of them,
which is what makes it package-wide. Two configurations for one adapter raise: they are
the one pairing with no defined order.

Because delivery is a ContextVar, a key set inside the block cannot leak across threads
or asyncio tasks — which is what makes it safe for a server or notebook handling several
users' credentials, the thing assigning to os.environ could never do.

The file

~/.dataretrieval/config.toml, or the path in DATARETRIEVAL_CONFIG. Top-level keys are
package-wide; [<adapter>] is that adapter's default profile, always in effect;
[<adapter>.<name>] is a named profile, inert until a caller selects it, so adding
one never changes an existing script:

api_key = "..."                 # package-wide
retries = 6

[waterdata]
concurrency = 16                # waterdata's default profile: always active

[waterdata.overnight]           # a named profile: only when selected
concurrency = "unbounded"
parallel_chunks = 8

[ngwmn.gentle]
concurrency = 2

Everything a profile does not name still comes from below it, per setting — so the
api_key and retries above are written once and inherited by both profiles. A file
containing a key that is readable by other users warns and prints the chmod to run.

Each adapter accepts only the settings it reads, and they are the fields of its
configuration class, so a setting that means nothing to a single-shot adapter is an
error rather than a line that quietly does nothing:

>>> WqpConfiguration(concurrency=2)
TypeError: WqpConfiguration.__init__() got an unexpected keyword argument 'concurrency'
ConfigurationError: …/config.toml: 'parallel_chunks' at [streamstats] is not a setting
that table accepts. It accepts: base_url, retries, stall_timeout.

Validation is lazy: a file's structure is checked when it is parsed, a table's keys when
that adapter first resolves a setting. That keeps a malformed [nldi] table from failing
a Water Data call, and it is what lets each schema live in a module the parser cannot
import.

Precedence

Seven rungs, highest first, applied per setting:

  1. A configuration instance passed to configure()
  2. A profile selected in code, WaterdataConfiguration.load("bulk")
  3. The setting's API_USGS_* environment variable
  4. The adapter's default profile in the file — [<adapter>]
  5. The package-wide keys at the top of the file
  6. The adapter's own built-in preference — NWDC asks for 4 concurrent requests
  7. The package built-in default

Rung 2 above rung 3 is the one place this inverts ADR 0009's environment-above-file rule.
A profile named in code is a more deliberate act than a variable inherited from whatever
started the process, and losing to that variable is what a caller would file a bug about.
The inversion covers what the profile names and nothing else. Rungs 1 and 2 both name a
single adapter and two configurations for one adapter raise, so they cannot tie; between
nested blocks the innermost decides.

Pointing an adapter at another host

An adapter's configuration may carry its base_url, which redirects that adapter's
requests and no other's — a staging instance, a mirror, a recording proxy:

with dataretrieval.configure(
    WaterdataConfiguration(base_url="https://staging.example/waterdata")
):
    df, md = waterdata.get_daily(monitoring_location_id="USGS-05114000")

It replaces that adapter's own base and the package appends its usual paths, so for Water
Data one value moves the OGC collections, the Samples database, the statistics service and
the STAC catalog together. NGWMN is served from the same host and is deliberately left
where it was.

Code only. A file that silently redirected a data-retrieval library to another host
would be a supply-chain-shaped hazard, and a variable that was quietly ignored would leave
a caller believing they had redirected something. Both refuse out loud:

ConfigurationError: …/config.toml: 'base_url' at [waterdata] may only be set in code,
in a configure() block, never from a file.

ConfigurationError: $API_USGS_BASE_URL is set, but 'base_url' may only be set in code,
in a configure() block, never from the environment. Unset it and pass the value on the
adapter's configuration, e.g. WaterdataConfiguration(base_url=...).

The API key does not follow. It is scoped to the single host that honours it, so a
redirected call goes out without it — the host you redirected to is not the host you gave
a credential to.

Introspection

show_configuration() names the exact source of each value, including which table of the
file and — when a caller selected one — which profile. It never prints the key, and it
never raises, because a broken configuration is exactly when you reach for it. The sample
below is generated by running the function; a test rebuilds the scenario and compares the
output verbatim against both the docstring and the user guide:

>>> with dataretrieval.configure(WaterdataConfiguration.load("bulk")):
...     dataretrieval.show_configuration()
config file  /home/u/.dataretrieval/config.toml (found)
api_key          <set>  /home/u/.dataretrieval/config.toml
concurrency      16     /home/u/.dataretrieval/config.toml
retries          8      $API_USGS_RETRIES
progress         auto   built-in default
parallel_chunks  1      built-in default
stall_timeout    60s    built-in default

A built-in default is package-wide. An adapter may prefer its own for
its own calls; a value from any source above overrides both.

adapter overrides
  waterdata  parallel_chunks  8  configure() block [waterdata.bulk]
  ngwmn      concurrency      4  /home/u/.dataretrieval/config.toml [ngwmn]

profiles in the file: [waterdata.bulk]
  A profile applies only where a row above names it; select one in
  code with <Adapter>Configuration.load("<name>").

not reported: nldi (not imported, so the settings each accepts are unknown here)

The profile section lists what the file defines whether or not this run selected any of
it, which is the answer to "I added a profile and nothing changed". The last line is the
honest cost of lazy validation: NLDI is imported on demand for the geopandas extra, so a
process that has not touched it cannot say what it accepts — and an omitted service would
read as "nothing is configured for it", which is a different claim.

A worked example

The same statewide pull under two profiles, timed. On this branch (pip install -e .),
paste into a file and run:

import os, pathlib, tempfile, time

# Throwaway config file, so this never touches ~/.dataretrieval/config.toml.
cfg = pathlib.Path(tempfile.mkdtemp()) / "config.toml"
cfg.write_text("""
[waterdata.plain]
parallel_chunks = 1     # split only as far as the URL byte limit forces

[waterdata.bulk]
parallel_chunks = 16    # fan the same query out into 16 sub-requests
""")
os.environ["DATARETRIEVAL_CONFIG"] = str(cfg)

import dataretrieval
from dataretrieval import waterdata
from dataretrieval.waterdata import WaterdataConfiguration

# Every stream gage in Delaware (~250) — few enough that the default plan is a
# single sequential page walk, so the profile's fan-out is what changes.
sites, _ = waterdata.get_monitoring_locations(
    state_name="Delaware", site_type_code="ST"
)
ids = sites["monitoring_location_id"].tolist()

def timed(profile, window):
    start = time.monotonic()
    with dataretrieval.configure(WaterdataConfiguration.load(profile)):
        df, _ = waterdata.get_daily(
            monitoring_location_id=ids, parameter_code="00060", time=window
        )
    print(f"{profile:6s} {window}  {len(df):>7,} rows  {time.monotonic() - start:5.1f}s")

# Two different decades on purpose: the API caches by data window, so re-running
# one window would serve the second call from cache and hide the difference.
timed("plain", "1985-01-01/1994-12-31")
timed("bulk", "1995-01-01/2004-12-31")

Measured against the live API:

plain  1985-01-01/1994-12-31   51,391 rows    9.2s
bulk   1995-01-01/2004-12-31   68,710 rows    2.7s

Same query shape, cold windows on both sides — bulk returned 34% more rows in less than
a third of the time. The only difference is which profile was selected.

Breaking changes

Nothing here has shipped, so nothing is deprecated.

configure() no longer takes keywords. configure(api_key=...) and the per-adapter
mappings configure(ngwmn={"concurrency": 4}) are gone, along with the public
WaterdataSettings / NgwmnSettings / … TypedDicts that annotated them. Write
Configuration(api_key=...) and NgwmnConfiguration(concurrency=4). Passing anything
that is not a configuration raises and names the replacement, so an old script says what
to write rather than failing obscurely. This is the most-typed line the feature exists to
enable and making it wordier is a real cost, accepted deliberately for one shape
everywhere.

The global profile table is retired. [profiles.<name>], DATARETRIEVAL_PROFILE and
configure(profile=...) are gone; a profile now belongs to one adapter. A file still
using the old table says what to write instead:

ConfigurationError: …/config.toml: [profiles] is no longer read. A profile now belongs
to one adapter: write [<adapter>.<name>] and select it with
<Adapter>Configuration.load("<name>").

dataretrieval.configdataretrieval.configuration, with no alias — the concept is
spelled in full everywhere else. The user-facing ~/.dataretrieval/config.toml is
unaffected and keeps its name.

RetryPolicy.from_env()RetryPolicy.from_configuration(). It now reads a
configure() block and the config file too, so a name saying "environment" is exactly the
drift this PR exists to prevent.

dataretrieval.waterusedataretrieval.nwdc. Every other adapter is named for the
service it retrieves from — ngwmn, nldi, wqp, streamstats, nwis — and this one
was named for a subset of what its service offers. The National Water Availability
Assessment Data Companion serves ten modeled datasets; the water-use models are five of
them, the rest hydrologic ensembles, WRF atmospheric forcing, and CONUS 2025 assessment
outputs:

$ curl -s https://api.water.usgs.gov/nwaa-data/ | jq -r .message
Welcome to National Water Availability Assessment Data Companion v2.0.0 API

dataretrieval.wateruse remains as a deprecated alias until 2027-08-11 or later,
following the dated-removal convention nwis uses. It re-exports rather than copies, so
wateruse.get_wateruse is nwdc.get_wateruse. It is an alias for reading, not a second
name for the module: assignment and private names do not forward, and the docstring says
so. import dataretrieval stays silent — the package imports nwdc directly, so only
code naming wateruse sees the warning. Public function and constant names are unchanged.

Decision records

ADR 0009 records the layered chain. ADR 0010 scopes settings per adapter, since
surveying all seven APIs showed ADR 0009's premise — that every service accepts the same
settings — is false. ADR 0011 makes a profile a named set of settings for one adapter
and moves each schema into the module that reads it; it supersedes two clauses of ADR 0010
and three of ADR 0009, each marked in place rather than rewritten.

ADR 0010's credential finding was settled by measurement rather than argument, and
reversed the assumption we started from. NGWMN is served from the Water Data host, so it
already receives that key. Probing both live: each returns 200 with no key and a
rate-limit header with one, and alternating authenticated calls decrement a single
counter (997, 996, 996, 994, 993, 992). One key, one quota pool, two adapters. Water
Data's OpenAPI declares ApiKeyHeader; NGWMN's declares no security scheme across 34
paths, yet the gateway meters it anyway (via: … api-umbrella on every response). The key
belongs to the gateway fronting the host, not to either adapter — so per-adapter keys
would model a distinction that does not exist, and credentials keeps its host scoping
unchanged.

ADR 0011 re-probed all three hosts and narrowed that further: NWDC and NLDI meter by
address and report the same limit with or without a key, and the three hosts keep
independent counters. Sending the key there would gain nothing and would turn a stale key
into 403s on calls that work anonymously today.

The deprecated nwis deliberately gets no table: its calls pin max_retries=0, so one
could only be reported as live and then ignored.

Also included

Bug fix: API_USGS_STALL_TIMEOUT was read straight from os.environ, so it could not
be set by a configure() block or the config file and never appeared in
show_configuration() — a gap in ADR 0009's own claim that every setting resolves through
one chain. stall_timeout now resolves like the rest, and configuration is the only
module left that reads the environment for a setting.

ssl_check deliberately stays a per-call argument. It disables certificate verification,
so a config key would make a security downgrade process-wide and invisible at the call
site — and its legitimate use, a TLS-intercepting proxy, is served better by
SSL_CERT_FILE, which httpx honours on both sync and async clients and which therefore
covers every getter including the OGC ones that have no ssl_check at all. The
configuration guide documents it.

CONTEXT.md gains the configuration vocabulary and separates built-in default
(package-wide) from adapter default (what an adapter supplies in code), because they
are not the same number and show_configuration() should not imply otherwise. The
**queryables passthrough still refuses credential-shaped keyword names before request
construction, now naming the current spelling in the error a caller sees.

Verification

949 tests · mypy --strict clean across 59 files · 7/7 import contracts · 14 pre-commit
hooks · docs build adds no new warnings. The Windows path resolution is covered
specifically: _home_id mirrors expanduser per platform, since ntpath reads
USERPROFILE then HOMEDRIVE+HOMEPATH and ignores HOME entirely — where Git Bash and
MSYS both set it.

@thodson-usgs

Copy link
Copy Markdown
Collaborator Author

@davetapley, this PR is AI generated but I'm happy to incorporate any high level feedback. Focus on the public interface for now.

@thodson-usgs
thodson-usgs force-pushed the worktree-config-fallback-352 branch from 6388881 to 399bfc4 Compare August 6, 2026 19:30
@davetapley

Copy link
Copy Markdown
Contributor

@thodson-usgs thanks for the fast reply! The configure block is definitely be my preference.

Config file has the same problem as env var: yes, technically possible to write it during runtime, but feels like it should be set up before the program runs, which is clunky if invoking from other parts of a different codebase.

@thodson-usgs

thodson-usgs commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

I considered using a class: with Configuration:, which is a pattern I've seen in other libraries. Claude didn't like it for this case, but I plan to reconsider before implementing.

@thodson-usgs thodson-usgs added the enhancement New feature or request label Aug 7, 2026
@thodson-usgs
thodson-usgs force-pushed the worktree-config-fallback-352 branch 4 times, most recently from 356a2f1 to f5dfca8 Compare August 11, 2026 15:30
@thodson-usgs thodson-usgs changed the title feat(config): resolve settings through a layered chain feat(config)!: resolve settings through a layered chain Aug 11, 2026
Settings reached the library through process-global environment variables
only, each with its own parser at its point of use. That cannot express a
per-call credential: an application holding keys in a secret store, or a
service handling concurrent users, had to assign to os.environ, which races
across threads and tasks (DOI-USGS#352).

Add dataretrieval.config, a stdlib-only leaf resolving every setting through
one ordered chain, highest precedence first: an active configure(...) block
(a ContextVar, so it cannot leak across threads or tasks), the setting's
environment variable, the TOML file at ~/.dataretrieval/config.toml, then the
built-in default. Precedence applies per setting, so a file that sets only
`concurrency` leaves an environment API_USGS_PAT in effect. The environment
sits above the file to keep the original interface authoritative; rationale
in ADR 0009.

The module owns each setting's grammar, so a value means the same thing
whichever source wrote it, and show_configuration() reports the effective
value and provenance of each without printing the key.

Consequences at the read sites:

- RetryPolicy.from_env becomes from_configuration. It reads a configure()
  block and a config file too, so a name saying "environment" is the drift
  this change exists to prevent.
- Concurrency resolves through the chain instead of reading the environment
  in transport.fanout, so the rule that an explicit setting outranks an
  adapter preference lives in one place rather than being restated.
- ConfigError is dropped in favor of the taxonomy's existing
  ConfigurationError -- same bases, same purpose, and only one has shipped.
- default_headers checks the host before resolving the key. Resolution now
  reads a file and can raise, so the old order let a Water Data config
  problem break an NWIS, WQP, or NGWMN call that would never have received
  the key.
- parallel_chunks(n) becomes sugar for configure(parallel_chunks=n) rather
  than owning a second ContextVar, so show_configuration() always reports the
  value the chunker will use.
- ChunkedCall.resume resolves the concurrency cap per drive rather than
  fixing it at construction: it is the one dial a caller adjusts precisely
  while retrying, so a configure() block entered between the interruption and
  the resume has to win.

_home_id mirrors expanduser per platform: posixpath reads HOME, while ntpath
reads USERPROFILE then HOMEDRIVE+HOMEPATH and ignores HOME entirely. The
POSIX ordering made the memo watch a variable that cannot move the resolved
path on Windows -- where Git Bash and MSYS both set HOME.

Settings are scoped per adapter (ADR 0010)
------------------------------------------

ADR 0009 assumed one flat namespace, on the premise that a setting means the
same thing to every service. Surveying all seven APIs showed the premise is
false: concurrency and parallel_chunks are meaningless to the four
single-shot adapters, which have nothing to fan out, and a flat namespace
accepted configure(streamstats={"parallel_chunks": 8}) without complaint --
the typo class ADR 0009 exists to catch.

A [ngwmn] table in the file, or configure(ngwmn={...}), now applies to that
adapter alone, so one block can be gentle with NGWMN while leaving Water Data
untouched. A profile could not express this: it switches a whole
configuration, not one service's slice. An adapter table overrides the
top-level one per setting, inheriting everything it does not name.

Precedence stays source-major -- block, then environment, then file, with an
adapter-scoped value outranking a package-wide one only within the same
source. Scope-major ordering would have inverted ADR 0009's
environment-above-file rule the moment anyone added a table, letting a stale
[wqp] entry beat a variable exported for one run.

Each adapter is a named, typed parameter on configure() carrying its own
TypedDict, so mypy --strict rejects a setting that adapter does not read, by
schema name, before the code runs. The remaining **unknown catch-all exists
only to turn a misspelled setting -- configure(concurrancy=8) -- into a
message naming the settings rather than a bare TypeError.

api_key is absent from every schema, and that was settled by measurement
rather than argument. NGWMN is served from the Water Data host, so it already
receives that key; probing both live, each returns 200 with no key and
x-ratelimit-limit: 1000 with one, and alternating authenticated calls
decrement a single counter (997, 996, 996, 994, 993, 992). One key, one quota
pool, two adapters. The key belongs to the gateway fronting the host, so a
per-adapter key would model a distinction that does not exist, and
credentials keeps its host scoping unchanged. progress is likewise
package-wide: there is one progress line per call.

The deprecated nwis has no table. Its calls pin max_retries=0, so a [nwis]
entry could only be reported as live and then ignored -- the failure this
decision exists to prevent.

stall_timeout joins the chain. API_USGS_STALL_TIMEOUT was read straight from
os.environ, so it could not be set by a block or the file and never appeared
in show_configuration() -- a gap in ADR 0009's own claim that every setting
resolves through one chain. transport/env.py existed only to parse it and is
deleted; config is now the only module that reads the environment for a
setting.

ssl_check deliberately does not join it. It disables certificate
verification, so as a per-call keyword it is a visible, scoped decision,
while a file key or environment variable would make a security downgrade
process-wide and invisible at the call site. Its legitimate use -- a
TLS-intercepting proxy -- is already served better by SSL_CERT_FILE, which
httpx honors on both its sync and async clients, so that covers every getter
including the OGC ones that have no ssl_check, and trusts the corporate CA
rather than trusting nothing. The configuration guide documents it.

Renames dataretrieval.wateruse to dataretrieval.nwdc
----------------------------------------------------

Every other adapter is named for the service it retrieves from -- ngwmn,
nldi, wqp, streamstats, nwis. This one was named for a subset of what its
service offers: the National Water Availability Assessment Data Companion
serves ten modeled datasets, of which the water-use models are five.

dataretrieval.wateruse remains a deprecated alias until 2027-08-11 or later,
per the dated-removal convention nwis uses. It re-exports rather than copies,
so wateruse.get_wateruse IS nwdc.get_wateruse. It is an alias for reading,
not a second name for the module: assignment and private names do not
forward, and the docstring says so. `import dataretrieval` stays silent --
the package imports nwdc directly, so only code naming wateruse sees the
warning, pinned by a subprocess test because an import side effect cannot be
tested honestly in an interpreter that already imported the package.

906 tests, mypy clean across 58 files, 7/7 contracts, all 14 hooks.

BREAKING CHANGE: RetryPolicy.from_env() is renamed to
RetryPolicy.from_configuration(), and resolves through the full chain rather
than the environment alone.

BREAKING CHANGE: dataretrieval.wateruse is renamed to dataretrieval.nwdc.
The old module remains as a deprecated alias emitting a DeprecationWarning on
import, and will be removed on or after 2027-08-11.

Closes DOI-USGS#352.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thodson-usgs
thodson-usgs force-pushed the worktree-config-fallback-352 branch from f5dfca8 to 7352053 Compare August 11, 2026 16:23
thodson-usgs and others added 18 commits August 11, 2026 12:02
An adapter-scoped block entry outranked a package-wide one unconditionally,
and the scope was one flat mapping with no notion of which block wrote a key.
So an *outer* configure(waterdata={...}) beat an *inner* configure(...),
inverting the nesting rule configure() documents.

Two user-visible consequences, both reproduced:

    configure(waterdata={"parallel_chunks": 2}):
        parallel_chunks(16)          -> resolved 2, not 16

    configure(waterdata={"concurrency": 32}):
        configure(concurrency=1)     -> resolved 32, not 1

The first silently discarded a per-call request: parallel_chunks(n) is sugar
for configure(parallel_chunks=n), which writes the package-wide key, so any
enclosing adapter table outranked it. The second made the documented recovery
from QuotaExhausted -- wait, then re-issue more gently -- inexpressible once
any adapter table was in play.

Each scope entry now carries the depth of the block that wrote it. An
adapter-scoped value outranks a package-wide one only at the *same* depth,
where one block named both and the adapter is the more specific of the two; a
deeper package-wide value was written by an inner block and wins. Same-source
precedence between the two scopes is unchanged for the case it was designed
for, and nesting behaves as documented again.

Found by the xhigh review, which flagged both and whose verifiers reproduced
them independently; the earlier /simplify pass had blessed the flat mapping as
correct, which it was for merging within one scope and not across two.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "check whether a leaf already generalizes it" rule named
`transport.retry._read_env_number` as the home for `API_USGS_*` settings. That
module is deleted: every setting now resolves through `config`, which is also
the only module left that reads the environment for one.

The rule is binding, so a stale entry is worse than a missing one -- a
contributor following it imports a symbol that does not exist, and the leaf
that would actually have answered their question goes unmentioned.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The docstring said "deliberately a leaf" after the module gained a
module-level import of `dataretrieval.config` (for `API_KEY_ENV`, so the
environment variable's name has one home). A reader acting on the stale claim
would remove that import to restore the property and break `API_KEY_ENV` at
import time.

Says what is actually true instead: config is its one first-party dependency,
is itself a standard-library-only leaf, and sits directly beneath this module
in the layers contract. It supplies the key's value; the questions this module
owns -- which host may receive it, and how it is withheld from every other --
are unchanged.

Also drops the function-local `import config` in `api_key()`, which duplicated
the module-level import and carried a comment explaining a laziness that no
longer applied.

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

configure() grew seven parameters this branch documents nowhere: the six
per-adapter tables and stall_timeout. The rendered API reference showed a
signature it never described, so the headline feature of ADR 0010 was
undiscoverable from the docs -- a caller had to read the ADR or the source to
learn that configure(ngwmn={...}) exists at all.

The adapter entry states what a reader cannot infer from the signature: that a
table overrides the package-wide value per key, that each adapter accepts only
the settings it reads and is annotated so a type checker says which, and why
api_key, progress and nwis are absent.

Also sharpens the module docstring's precedence paragraph, which said an
adapter-scoped value outranks a package-wide one "within the same source" --
true, but no longer the whole rule now that block entries carry their nesting
depth. Within the block source the tie-break is per block, and a nested block
still wins over both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six docstrings across three modules still presented the environment variable
as *the* mechanism for concurrency and retries, which stopped being true when
those settings began resolving through the chain -- and stopped being the
whole story again when they became scopable per adapter.

The most user-visible was `nwdc.get_wateruse`, whose own docstring told a
reader that `API_USGS_CONCURRENT` was how to change fan-out, so a caller
reading the getter they were calling would never learn that
`configure(nwdc={"concurrency": 2})` or an `[nwdc]` table exists. The rest are
the fan-out and chunking module docstrings and `FanOut.default_concurrent`,
which described the env var as the thing a service default fills in for.

Each now names the *setting* and points at the chain, mentioning the
environment variable as one source among several rather than the interface.
No behavior change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six cleanups from the xhigh review, none changing behavior except where noted.

show_configuration()'s docstring sample was missing two settings the function
always prints and the adapter-overrides section entirely, so a reader
comparing real output against the docs saw rows the docs denied. Regenerated
from actual output rather than edited by hand -- the same sample had already
gone stale twice by being edited.

The `keyword in SETTINGS` branch in _normalize_adapters was unreachable once
every setting became a named parameter: nothing that reaches the catch-all can
be a setting. Removed, and the surviving message says why anything arriving
there is unknown.

The unknown-setting warning hard-coded stacklevel=2 while _WARN_STACKLEVEL
exists to name that number once; changing the constant would have moved two of
three warnings.

_resolve loaded the config file twice on an adapter-scoped read -- once for
the adapter table, once for the top level -- so the common case (no table for
that adapter) paid an extra stat per setting. It now loads once and passes the
parsed file to both tiers: measured 2 stat calls -> 1 for
retries(adapter="waterdata"), and 4 -> 2 per RetryPolicy.from_configuration.

_show_adapter_overrides re-resolved each package-wide source label it needed,
though show_configuration had just computed all of them. Beyond the repeated
work, cell() mutates the shared error-dedupe state, so a broken config's
message could be consumed by a throwaway comparison before the row that needed
to print it. It now takes the labels already resolved.

tests/conftest.py drops the flaky_api marker and its rerun patterns. They are
deliberate, documented infrastructure, but nothing references them and they
have nothing to do with configuration -- they belong in the change that adds
the live tests using them, not in this one.

Not applied: the adapter roster appears in the TypedDicts, _ADAPTER_SCHEMAS
and configure()'s parameter list. Deriving any of those from another would
give up exactly what the explicit parameters buy -- a type checker naming the
schema in an error about a call -- so the repetition is the price of the
static check, not an oversight.

909 tests, mypy clean across 58 files, 7/7 contracts, all 14 hooks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d values

The precedence fix encoded each block's nesting depth into every value it
wrote, then re-derived that depth on entry by scanning the outer scope. Keeping
the frames themselves says the same thing with less machinery: "the innermost
block wins" falls out of iterating them in reverse, so the depth field, the
scan, the tuple unwrapping at every read site, and two comments restating the
tie-break rule all go away. _resolve returns to nearly its pre-fix shape,
wrapped in one loop. Net 11 lines lighter.

It also removes a subtlety worth not having: `max(...) + 1` computed "one more
than the deepest *recorded* depth", not the nesting level -- an enclosing block
that set nothing was invisible, so the stored number was only
decision-equivalent to depth, and a reader had to prove that before trusting
the comparison. `len(frames)` needs no proof. The depth carried on the profile
key was write-only state for the same reason.

And it undoes a regression the depth version introduced: entering a block was
O(all keys ever set), measured 18% slower at any nesting and 36% inside a
large outer scope. Appending a frame is O(1). Reads become O(nesting depth)
rather than O(1), which is 1-3 in practice and below the noise floor.

Behavior is unchanged and re-verified, including three-deep nesting and an
adapter block inside a different adapter's block, which the depth version was
never exercised against.

Also from the review:

- The user guide still stated the pre-fix rule -- that an adapter-scoped value
  outranks a package-wide one "only within the same source" -- which stopped
  being the whole truth for blocks. It was the one of three user-facing copies
  the fix missed, and this diff had just pointed nwdc's docstring at it.
- `_file_settings` took `path`/`parsed` as optional while its sibling
  `_adapter_file_settings` required them, so the file tier had two entry shapes
  and a caller could hand the two tiers different parsed files -- the drift the
  single-load change existed to remove. Both now take the pair, from one
  `_current_file()` helper.
- The `for keyword in unknown` loop always raises on its first iteration; it is
  an `if`, and now says so.
- configure()'s new docstring restated the api_key/progress rationale a fourth
  time. It points at ADR 0010 and keeps only the `nwis` clause, which appears
  nowhere else.

909 tests, mypy clean across 58 files, 7/7 contracts, all 14 hooks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Swept into the squashed commit by a `git add -A`, the same way CONFIG-PLAN.md
was. 4549 lines of resolver output in a library whose pyproject declares
ranges and which is not deployed from a pinned set, so it constrains nothing
and dates immediately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ADR 0011 proposes per-adapter configuration profiles, class-based configure(),
schemas owned by adapters with names held centrally, and the precedence ladder
that follows -- including the one inversion of ADR 0009's environment-above-file
rule, for a profile a caller selects in code.

CONTEXT.md gains 'configuration profile' (short: configuration), 'default
profile' vs 'named profile', 'effective configuration' for the resolved result,
and 'selection'. The config-as-abbreviation rule is withdrawn.

Model first, before the code that implements it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Prefactor for the configuration-profile work: the concept is spelled in full
everywhere else, and ADR 0011 withdraws the rule that reserved `config` as an
abbreviation for the module and the file. The path has never been released, so
no alias and no deprecation.

The user-facing file keeps its name -- `~/.dataretrieval/config.toml` is a
separate decision nobody has made, and renaming it would break every example
for no gain.

No behaviour change; 909 tests, mypy clean, 7/7 contracts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A configuration profile is now a named set of settings for ONE adapter, and
``configure()`` takes those configurations positionally instead of keywords
(ADR 0011).

The adapter a configuration targets is a property of its class, so a caller
never restates it -- which is what removes the adapter roster from every call
site. Each adapter owns its class, defined in the module that *reads* those
settings; ``dataretrieval.configuration`` keeps only the adapter names, which
is what parsing a file needs and all a standard-library-only leaf can hold.

  with dataretrieval.configure(
          Configuration(api_key=vault.read("usgs/pat")),
          WaterdataConfiguration.load("bulk"),
          NgwmnConfiguration(concurrency=4),
  ):
      ...

- ``BaseConfiguration``: frozen dataclass, all fields optional, ``adapter``
  ClassVar, a ``validate()`` hook for rules no single setting can express,
  ``settings()``, ``values()``, and ``load(profile)``. Values are checked at
  construction, so a typo raises on the line that wrote it.
- ``Configuration`` carries the package-wide settings; ``ADAPTERS`` holds the
  roster and ``_register`` populates a registry at adapter import.
  ``settings_for()`` returns None for an adapter this process has not imported
  -- "cannot validate these keys yet", never an error, since nldi is
  import-on-demand for the geopandas extra.
- The file gains named profiles beside each adapter's default profile:
  ``[waterdata]`` always applies, ``[waterdata.bulk]`` only when selected, and
  a named profile inherits the tiers below it per key. Selecting one is code,
  so it outranks the setting's environment variable -- the one place ADR 0011
  inverts ADR 0009.
- An adapter configuration may carry ``base_url``, settable in a block and
  refused from the file and the environment: a file that silently redirects a
  data-retrieval library to another host is a supply-chain-shaped hazard.

Breaking, none deprecated because nothing shipped: ``configure(api_key=...)``,
the ``configure(ngwmn={...})`` mappings and their public ``*Settings``
TypedDicts, the global ``[profiles.<name>]`` table and ``DATARETRIEVAL_PROFILE``
are all gone. A file still using the retired table gets an error naming its
replacement. ``show_configuration()`` drops its ``profile`` row and now names
any adapter it could not cover rather than omitting it -- the honest cost of
validating an adapter's keys lazily.

Tests migrated to the new API; those exercising the retired global profile
table are deleted. Added: same-adapter collision, a non-configuration
argument, a package-wide configuration resolving end to end through every
tier, an adapter configuration narrowing to one adapter, named profiles
selected independently per adapter, a code-selected profile beating the
environment, base_url from code and refused from the file, and the
``validate()`` hook.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTaSm7HmVb94RSJiKW4WAS
ADR 0011's file grammar -- top-level scalars, a ``[<adapter>]`` default
profile, and inert ``[<adapter>.<name>]`` named profiles -- landed with
the configuration-object refactor but was only partly pinned by tests.
Cover each rule the grammar promises, and close the one hole the coverage
found.

A table nested inside a named profile was silently dropped, so a file
migrated from the retired ``[profiles.bulk.ngwmn]`` -- where a profile
did carry per-service detail -- would look like it had tuned NGWMN and
tune nothing. Selecting such a profile now raises and names the shape to
write instead. The check is lazy, like every other key check on a
profile: a malformed profile for one adapter must not fail another
adapter's call.

Tests added for: a named profile layering per key over the default
profile and the package-wide keys; a file gaining a profile changing no
resolved setting until code selects it; a nested table inside a profile;
``DATARETRIEVAL_PROFILE`` staying retired rather than quietly honored
under the new grammar; a malformed ``[nldi]`` table costing a Water Data
call nothing; and a ``[nldi]`` table staying valid while nldi has never
been imported.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTaSm7HmVb94RSJiKW4WAS
…file

ADR 0011's ladder was implemented by the file-grammar step but only
partly pinned: the suite asserted rungs 1, 3 and 5 and one instance of
2-beats-3, so a refactor that fused two rungs could still pass. Verified
each of the ADR's requirements against the code and a live probe, then
covered the ladder as a chain -- each test knocks out the rung above and
asserts the next takes over.

`nwdc` and `concurrency` are the pair that can express all seven rungs:
NWDC is the one adapter shipping a built-in preference of its own (4)
distinct from the package default (32), which is the only way rungs 6
and 7 can be told apart at all. Resolution goes through the adapter's
own read site so that preference is really in the chain.

Confirmed non-vacuous by mutation: moving the environment above the
configure() frame in _resolve fails the 2-beats-3 test, dropping the
[<adapter>] tier fails both rung-4 tests, and ignoring the read site's
`default` fails both rung-6 tests.

Also covered: rungs 1 and 2 cannot tie, because both name one adapter
and the same-adapter rule refuses the pairing -- with the neighbouring
case a reader of the ladder gets wrong, a package-wide Configuration
beside a loaded profile, where the adapter-scoped value wins for that
adapter and the package-wide one still governs the rest. And load()
itself: it returns an instance of the class it was called on carrying
only the profile's keys, and an undefined name lists the names the file
does define, for that adapter alone.

No behaviour change. The docs said precedence in four tiers where the
ADR says seven, so the user guide now spells all seven -- the tier it
was hiding is an adapter's own built-in preference -- and the module
docstring says how its four sources map onto them.
show_configuration() could say a value came from a configure() block but
not *which* configuration put it there, so a caller who selected the wrong
profile -- or who had forgotten one was selected -- had nothing to look at.
A loaded configuration now remembers the profile it was read from, and each
block override carries the label naming it, so a row reads
``configure() block [waterdata.bulk]``: the table's own spelling, greppable
in the file that defines it.

The label is built in ``_frame`` rather than at resolution time because
that is the last point where the profile is known -- values from
``load("bulk")`` and from ``WaterdataConfiguration(...)`` are
indistinguishable once they are in the frame. Frames therefore hold
(value, source) pairs, the shape the file tier already returned.

The report also lists the named profiles the file defines, selected or
not. A named profile changes nothing until a caller selects it, which is
the thing readers of a file get wrong; without the line, "I added
[waterdata.bulk] and nothing changed" has nothing to explain it. Names come
from the parsed file, so an unimported adapter's profiles are listed too --
what a table means needs the import, what it is called does not. Nothing in
the section validates, so a malformed profile is reported rather than
taking the report down with it.

The unimported-adapter caveat moves to its own printer, keeping section
order (overrides, profiles, caveat) and leaving each function one section.
Behaviour there is unchanged: nldi is named, never omitted.

Both documented samples were regenerated by running the function -- the
docstring had wrapped a line the function prints whole, and the guide had
lost a paragraph. A new test rebuilds the scenario, compares the captured
output verbatim, and fails when either drifts, since a sample kept by hand
is only as fresh as the last person who remembered it.
Every adapter configuration already carried a base_url; nothing read it.
Each adapter now resolves its base per call -- ``base_url(adapter=...) or
<the service's own>`` -- at the one place its request URLs are built, so a
``configure`` block redirects that adapter and no other. Resolution is per
call rather than at import because a block is scoped to a ``with``
statement and delivered through a ContextVar; a constant computed once
could only describe the process.

Water Data serves four APIs from one root, so its value names that root and
``endpoints.redirected()`` moves the OGC collections, the Samples database,
the statistics service and the STAC catalog together -- a redirect that
reached only the family a caller happened to call first would send the rest
to the host they were redirecting away from. WQP's two interfaces move
together for the same reason. NWDC threads the resolved host into its page
walk as well: its ``water.usgs.gov`` alias list and host rewrite are facts
about that service, so a redirected call gets the general rule instead
(follow a link only back to the host that served the page), rather than
having page two rewritten onto the USGS.

The environment is now refused out loud instead of merely unread:
``API_USGS_BASE_URL`` is the spelling every other setting's variable
predicts, so exporting it and being silently ignored would leave a caller
believing they had redirected something. It raises before any source is
consulted, matching the file, which raises whether or not a block also set
one -- a variable that cannot work must not be quietly outranked by a block
that happens to work, since it survives to a run where nothing outranks it.

The API key needs no new rule to stay put: credentials checks the host when
attaching it, so a redirected adapter simply does not receive it. Tested
anyway, with a control on the same key and the same call to the real host,
because "needs no rule" is the kind of claim that stops being true silently.

Tests: a code override moving real traffic for two adapters with unrelated
request machinery (OGC engine, one-shot GET), the four Water Data families
moving together, the environment refusal (including that
show_configuration() reports it rather than raising), NGWMN redirected
alone while Water Data stays on the host they share, the NWDC page walk
following the mirror, WQP's two interfaces, NLDI's catalog probe and query,
and the credential not following a redirect. Each was proved non-vacuous by
mutating the source and reverting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTaSm7HmVb94RSJiKW4WAS
`configure(api_key=...)` no longer exists, so every place that showed it, or
the per-adapter `configure(ngwmn={...})` mappings, was wrong. This is the
documentation half of that change; no behaviour moves.

The configuration guide now leads with the case the feature exists for --
one block, three services, two loaded from the file by name and one built in
code -- instead of burying it under "Named profiles" as a second example.
That section keeps only what is specific to the file grammar and `load()`,
and points back rather than restating a worked example. The example was run
before it was written: the resolved values in the prose are what the chain
actually produces for that file, including the two that are easy to get wrong
(WQP departs from the file's `retries`, and the key reaches only the host
that honours it). The `configure` block section gains a note naming the
retired spellings, since that is what a reader of an old script will search
for.

ADR 0011 moves to Accepted: every item in its own Compliance list is covered
by a shipped test, so the section now names them instead of asking for them.
ADRs 0009 and 0010 are marked, not rewritten -- their Status sections say
which clauses 0011 supersedes, and the clauses themselves carry a marker
where a reader could otherwise act on superseded text (0010's decisions 5 and
8, the `configure()` spelling in its decision 1, and the two consequences
that turned on refusing a configuration object). Broken pointers left by the
`config` -> `configuration` rename are fixed in place; no decision text is
edited to reverse its meaning.

`show_configuration()`'s sample is unchanged and re-verified by running it:
`test_show_configuration_sample_output_is_current` rebuilds the scenario and
compares the captured output verbatim against both the docstring and the
guide.

Also: the `**queryables` credential refusal now names the current spelling in
the TypeError a caller actually sees, and two comments in `nwdc` and
`transport.fanout` stop illustrating adapter scope with a keyword that is
gone.

949 tests, mypy --strict clean, 7/7 contracts, pre-commit clean, docs build
adds no new warnings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTaSm7HmVb94RSJiKW4WAS
The migration removed one duplication (the adapter roster at every call
site) and introduced several smaller ones. This collapses them, and closes
the two seams the review found guarded only by convention.

Duplication removed
- Shared setting groups. `retries` / `stall_timeout` / `base_url` /
  `concurrency` / `parallel_chunks` were each spelled once per adapter
  class, with verbatim docstrings and nothing keeping them in step -- the
  annotations are decorative, since `_coerce_typed` keys the type check by
  setting *name*, so a drift to `retries: str | None` type-checked clean.
  Each is now declared once as `_Retrying` / `_Redirectable` / `_Concurrent`
  / `_Chunked` in `configuration`, and an adapter composes the groups it
  reads. *Which* groups is still the adapter's own knowledge, which is what
  ADR 0011 asked for.
- `SETTINGS` is derived from `fields(Configuration)` rather than restated,
  in the one module whose job is to stop rosters being duplicated. A field
  added and forgotten used to work from `configure()` and be silently
  dropped from the file.
- `_REFUSED_ENV_VARS` is derived from `ADAPTER_ONLY_SETTINGS`. The two
  tables spelled one fact -- "this setting is code-only" -- and a second
  adapter-only setting added to the roster alone would have been refused by
  the file and *silently ignored* from the environment, which is the defect
  the table was written to prevent.
- `base_url()` takes the fallback: `base_url(adapter="nldi",
  default=NLDI_API_BASE_URL)`. The `... or SERVICE_DEFAULT` rule was spelled
  at five read sites; the service's own URL still lives beside the service.
- `_checked_table()` is the one check-one-table routine; `_scalars` and
  `_named_profile` each take the half they need. They had been the same
  loop, including the source-label format, written twice.
- The credential-name predicate moves to the `credentials` leaf as
  `refuse_credential_keywords()`. The fact that motivates it -- `api_key=`
  is a plausible kwarg now that `configure()` takes it -- is package-wide,
  and WQP's ten getters forward `**kwargs` straight into the query payload.
  BEHAVIOR CHANGE, noted in NEWS: a credential-shaped WQP filter now raises
  `TypeError` rather than putting a secret in a URL.

Work removed
- `_frame()` renders with `_coerce_typed` instead of re-running the grammar
  `__post_init__` already ran. Construction is the single validation point.
- `parallel_chunks(n)` validates through `_validated_raw`, the table that
  owns the setting's bound, rather than a second grammar in `ogc.policy`.
  `ConfigurationError` is a `ValueError`, so the caller contract holds; the
  matched text in the test moves to the shared message.
- `settings()` is memoized on the class. Rebuilding a six-element frozenset
  from `fields()` was 17-19% of every adapter-scoped read, and `_accepts`
  asks for it before the frame walk and before the file. Keyed on the class,
  not the name, because tests replace registry entries.

Seams closed
- `_resolve` raises on an adapter name it does not recognize. A typo used to
  resolve silently package-wide, so a `[waterdata]` table would be ignored
  with nothing raised; the fitness test could only see that the correct
  string occurred somewhere.
- New AST fitness test asserts every Water Data endpoint constant reaches
  `redirected()`. Water Data is the one adapter that cannot resolve its base
  at a single choke point, and a bare use sends traffic to the host a caller
  redirected away from with nothing failing.

Also: `_WARN_AT_TOP_LEVEL` declares the `parallel_chunks` warning as data
beside the other per-setting policies rather than as a name test in a loop
body; `progress` imports `configuration` at module level like its two
siblings (no cycle exists); the Windows stamp-cache comment carries the
measured cost so the next reviewer does not re-derive it; NEWS collapses
three entries describing one unreleased feature's superseded designs into
one; and ADR 0011's `show_configuration()` consequence is reworded to what
shipped -- the profile list is deliberately not import-limited, and a test
forbids filtering it.

Skipped: the per-service `_api_base()` / `_service_base()` wrappers stay.
The duplication the review named was the fallback *rule*, which `base_url()`
now owns; deleting the wrappers would push the adapter name and the service
constant back out to ten call sites. Two performance findings and one
Windows finding were explicit no-ops.
CI pins ruff 0.16.1 and runs `ruff format --check .` over the whole tree,
which formats code blocks inside Markdown. The local pre-commit hook only
targets Python files, so a README example never reached it -- pre-commit
passing does not imply CI lint passing, which is how this got pushed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Absorbs DOI-USGS#368, DOI-USGS#369, and DOI-USGS#371. Eight files conflicted; three of the
resolutions were more than textual:

- ``wateruse.py``: main edited the implementation this branch had
  already renamed to ``nwdc.py``, so git conflicted the shim against
  it. Kept the shim and ported DOI-USGS#371's ``run_paginated`` migration into
  ``nwdc.py``, preserving ``adapter="nwdc"``.
- ``cql.py``: took main's version wholesale. This branch's two changes
  there (``redirected(OGC_API_URL)`` and ``adapter="waterdata"``) are
  subsumed by DOI-USGS#368 routing ``get_cql`` through
  ``waterdata.utils.get_ogc_data``, which already applies both.
- ``ratings.py``: took main's rewritten implementation and re-applied
  this branch's only contribution to it (``redirected(STAC_URL)``),
  plus adapter scoping on both drives.

Three breaks were semantic, not textual -- git merged them cleanly and
they would have failed at import or call time, because main added
callers of names this branch renamed or deleted:

- ``transport/pagination.run_paginated`` imported ``_CONCURRENCY_DEFAULT``
  (deleted here in favour of ``configuration.DEFAULT_CONCURRENCY``) and
  called ``RetryPolicy.from_env`` (renamed to ``from_configuration``).
  It now takes an ``adapter`` argument and threads it to both the retry
  policy and the executor, which is what makes per-adapter ``retries``
  and ``concurrency`` tables reach the three getters that use it.
- ``ogc/engine``'s ``cql_body`` branch (new in DOI-USGS#368) called
  ``from_env`` and dropped the adapter; both fixed.
- ``get_ogc_data`` gained ``cql_body`` from main and ``adapter`` here;
  both parameters kept.

969 passed, mypy --strict clean, all hooks including import-linter pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BTaSm7HmVb94RSJiKW4WAS
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow API keys to be provided without modifying API_USGS_PAT

2 participants