feat(config)!: resolve settings through a layered chain - #353
Merged
thodson-usgs merged 1 commit intoAug 15, 2026
Merged
Conversation
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
force-pushed
the
worktree-config-fallback-352
branch
from
August 6, 2026 19:30
6388881 to
399bfc4
Compare
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. |
Collaborator
Author
|
I considered using a class: |
thodson-usgs
force-pushed
the
worktree-config-fallback-352
branch
4 times, most recently
from
August 11, 2026 15:30
356a2f1 to
f5dfca8
Compare
thodson-usgs
force-pushed
the
worktree-config-fallback-352
branch
from
August 11, 2026 16:23
f5dfca8 to
7352053
Compare
This was referenced Aug 12, 2026
Every tunable setting -- the Water Data API key, fan-out concurrency, retries, the stall timeout, the progress line -- now resolves through one ordered chain instead of the environment alone: a `dataretrieval.configure(...)` block, then the setting's `API_USGS_*` variable, then `~/.dataretrieval/config.toml` (or the path in `DATARETRIEVAL_CONFIG`), 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 public surface is `dataretrieval.configuration`, over a private `_configuration_core` holding the model, grammar, and file caches: - `configure()` takes configuration objects positionally, at most one per adapter. Each adapter owns its class in the module that reads the settings (`WaterdataConfiguration`, `NgwmnConfiguration`, `NwdcConfiguration`, `WqpConfiguration`, `NldiConfiguration`, `StreamstatsConfiguration`) and accepts only the settings it reads, so `[streamstats] parallel_chunks = 8` is an error rather than a line that quietly does nothing. The block is delivered through a ContextVar (the shared `Ambient` leaf), so a credential set inside it cannot leak across threads or asyncio tasks (DOI-USGS#352). - The file gains named profiles beside each adapter's default profile: `[waterdata]` is always in effect, `[waterdata.bulk]` only when a caller selects it with `WaterdataConfiguration.load("bulk")`. A selected profile still inherits per setting, and a profile named in code outranks the environment -- the one deliberate inversion of the ladder. - An adapter's configuration may carry a `base_url` that redirects that adapter's requests for the duration of the block; the file and the environment refuse it. Water Data acquires endpoints through `waterdata.endpoints`, so one value moves the OGC collections, Samples, Statistics, and the STAC catalog together. - `show_configuration()` reports each setting's effective value and exact source -- naming the profile behind a value, listing the profiles a file defines, and naming unimported adapters -- without ever printing the key. - One parser per setting owns its grammar and one roster its type policy, so a value means the same thing whichever source wrote it. Breaking / behavior changes (details in NEWS.md): - `RetryPolicy.from_env()` is now `RetryPolicy.from_configuration()` and resolves through the whole chain; `transport/env.py` is retired. - A credential-shaped keyword in a getter's `**kwargs` passthrough (`api_key=`, `token=`, ...) raises `TypeError` naming `configure()` instead of putting a secret in a URL. - `API_USGS_STALL_TIMEOUT` now resolves through the chain like every other setting; it was previously read straight from `os.environ` and invisible to blocks, the file, and `show_configuration()`. `dataretrieval.wateruse` is renamed `dataretrieval.nwdc`, after the service, like every other adapter. The old name remains as a forwarding alias emitting a dated `DeprecationWarning` (removal on or after 2027-08-11, recorded in `_deprecation.REMOVALS`). Rationale in ADRs 0009-0011; vocabulary in CONTEXT.md; user guide at docs/source/userguide/configuration.rst. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thodson-usgs
force-pushed
the
worktree-config-fallback-352
branch
from
August 15, 2026 16:15
a5e2747 to
873e126
Compare
thodson-usgs
marked this pull request as ready for review
August 15, 2026 16:16
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #352.
Adds
dataretrieval.configuration: one ordered chain that resolves every setting, and aconfigure()block that scopes settings to a single call, a single thread, or a singleservice — 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, atmost one per adapter, and nothing else:
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.
Configurationtargets 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 threadsor asyncio tasks — which is what makes it safe for a server or notebook handling several
users' credentials, the thing assigning to
os.environcould never do.The file
~/.dataretrieval/config.toml, or the path inDATARETRIEVAL_CONFIG. Top-level keys arepackage-wide;
[<adapter>]is that adapter's default profile, always in effect;[<adapter>.<name>]is a named profile, inert until a caller selects it, so addingone never changes an existing script:
Everything a profile does not name still comes from below it, per setting — so the
api_keyandretriesabove are written once and inherited by both profiles. A filecontaining a key that is readable by other users warns and prints the
chmodto 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:
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 failinga 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:
configure()WaterdataConfiguration.load("bulk")API_USGS_*environment variable[<adapter>]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'srequests and no other's — a staging instance, a mirror, a recording proxy:
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:
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 thefile 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:
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:
Measured against the live API:
Same query shape, cold windows on both sides —
bulkreturned 34% more rows in less thana 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-adaptermappings
configure(ngwmn={"concurrency": 4})are gone, along with the publicWaterdataSettings/NgwmnSettings/ …TypedDicts that annotated them. WriteConfiguration(api_key=...)andNgwmnConfiguration(concurrency=4). Passing anythingthat 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_PROFILEandconfigure(profile=...)are gone; a profile now belongs to one adapter. A file stillusing the old table says what to write instead:
dataretrieval.config→dataretrieval.configuration, with no alias — the concept isspelled in full everywhere else. The user-facing
~/.dataretrieval/config.tomlisunaffected and keeps its name.
RetryPolicy.from_env()→RetryPolicy.from_configuration(). It now reads aconfigure()block and the config file too, so a name saying "environment" is exactly thedrift this PR exists to prevent.
dataretrieval.wateruse→dataretrieval.nwdc. Every other adapter is named for theservice it retrieves from —
ngwmn,nldi,wqp,streamstats,nwis— and this onewas 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:
dataretrieval.wateruseremains as a deprecated alias until 2027-08-11 or later,following the dated-removal convention
nwisuses. It re-exports rather than copies, sowateruse.get_wateruse is nwdc.get_wateruse. It is an alias for reading, not a secondname for the module: assignment and private names do not forward, and the docstring says
so.
import dataretrievalstays silent — the package importsnwdcdirectly, so onlycode naming
waterusesees 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
200with no key and arate-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 34paths, yet the gateway meters it anyway (
via: … api-umbrellaon every response). The keybelongs to the gateway fronting the host, not to either adapter — so per-adapter keys
would model a distinction that does not exist, and
credentialskeeps its host scopingunchanged.
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
nwisdeliberately gets no table: its calls pinmax_retries=0, so onecould only be reported as live and then ignored.
Also included
Bug fix:
API_USGS_STALL_TIMEOUTwas read straight fromos.environ, so it could notbe set by a
configure()block or the config file and never appeared inshow_configuration()— a gap in ADR 0009's own claim that every setting resolves throughone chain.
stall_timeoutnow resolves like the rest, andconfigurationis the onlymodule left that reads the environment for a setting.
ssl_checkdeliberately 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 thereforecovers every getter including the OGC ones that have no
ssl_checkat all. Theconfiguration guide documents it.
CONTEXT.mdgains 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**queryablespassthrough still refuses credential-shaped keyword names before requestconstruction, now naming the current spelling in the error a caller sees.
Verification
949 tests ·
mypy --strictclean across 59 files · 7/7 import contracts · 14 pre-commithooks · docs build adds no new warnings. The Windows path resolution is covered
specifically:
_home_idmirrorsexpanduserper platform, sincentpathreadsUSERPROFILEthenHOMEDRIVE+HOMEPATHand ignoresHOMEentirely — where Git Bash andMSYS both set it.