Repository navigation
ADR 0014 (proposed): Service declarations and OGC endpoints - #445
Draft
thodson-usgs wants to merge 20 commits into
Draft
thodson-usgs wants to merge 20 commits into
thodson-usgs wants to merge 20 commits into
Conversation
A datetime.date, datetime.datetime or pandas.Timestamp passed to a date
argument (time=[df.index.min(), None]) failed inside the formatter with
AttributeError ('endswith') or, for a lone datetime, TypeError (not
iterable), naming no argument. Read them like the equivalent string:
aware values convert to UTC, naive ones use the local zone as naive
strings do, a date is midnight of that day, and NaT is an open bound.
Any other type raises ValueError naming the argument.
Follow-up named in DOI-USGS#441.
…st form
_format_api_dates sent any single string containing "/" unchanged. The
same range spelled as the documented string ("2024-01-01T10:00:00/..")
and as the list (["2024-01-01T10:00:00", None]) could select different
data: the string skipped the local-to-UTC conversion of naive times, the
offset-to-Z conversion, and the date truncation for date-only
collections, and "not-a-date/also-bad" reached the service unvalidated.
Split the string on its "/" and format each side as a list element is
formatted, so an unreadable side raises ValueError naming the caller's
argument before any request. An empty side becomes "..", and "../.."
means no filter, as [None, None] does. One side may still be an ISO 8601
duration paired with an instant ("2024-01-01/P7D"), kept unchanged; any
other shape (three sides, two durations, a duration with an open end)
raises ValueError naming the argument. A lone duration ("P7D") is
unchanged.
Follow-up named in DOI-USGS#441.
_validate_time_no_duration iterated a lone Timestamp/date and raised TypeError before the formatter could read it; it now normalizes with _coerce_to_list and checks only string elements. Also: drop the redundant date check in _coerce_to_list (a date is not iterable), update the Mapping TypeError to name the accepted types, and tighten the NEWS entry.
The Water Data and NGWMN services answer '2024-01-01/P7D' and 'P7D/2024-01-08' with HTTP 400, so passing them through only deferred the failure to an error that names no argument. _split_interval now rejects them and hands the two sides to the list path, which removes _format_interval's duration bookkeeping and the one-line _is_passthrough wrapper. Tighten the NEWS entry.
A tuple keeps _split_interval assignable to items once DOI-USGS#442 widens the element type, where a list[str | None] would trip list invariance.
The NGWMN and STAC firewalls answer any datetime containing '../' with HTTP 403, so get_water_level(datetime=[None, end]) and get_ratings(time=[None, end]) never worked; and since DOI-USGS#443 formats a '/end' string, the empty-start workaround stopped working too. The Water Data API is the reverse: it accepts '../end' and answers '/end' with HTTP 400. OgcDialect.open_start records the spelling (default '..'); NGWMN sets '', and get_ratings passes '' to _format_api_dates directly since STAC has no dialect. An open end stays '..', which all three accept.
… values Replaces the OGC-only proposal in DOI-USGS#444. Six abstractions (service, endpoint, protocol, dialect, execution, adapter), the rule that shared code names no service, dates read once and written per protocol, and values-not-clients. Proposed and revised as each implementation step lands. Adds the Service declaration, Endpoint, and Protocol glossary entries and restates Adapter and Dialect.
Each adapter now declares Service(name, default_url) once, in the module that owns its URL, and every read site takes the name from it: the Configuration class's adapter, retry/fan-out/pagination adapter=, and the base URL through configuration.service_url(). Water Data declares one service in waterdata.endpoints for its four APIs. No behavior change. A fitness function fails if a service's name is written as a literal anywhere but its declaration and the configuration roster; the read-site check now follows the declaration.
Correct the ADR's call-site counts (six adapters, 18 sites in 10 files) and say plainly that execution takes service.name; keep the TYPE_CHECKING import out of configuration's alias inventory; rename nwdc's local service_url, which shadowed the new function; note why Water Data's declaration is package-public; mark DateDialect in the glossary as proposed.
…014) Endpoint(service, url, dialect) is built by the Water Data and NGWMN adapters when a call starts. get_ogc_data, _construct_api_requests, and _finalize_ogc take it in place of the base_url, dialect, and adapter they used to receive separately; settings resolve by endpoint.service.name. DEFAULT_DIALECT, now unused, is removed. Tests: the engine reads URL, dialect, and settings from the endpoint; a call interrupted inside configure(base_url=...) resumes against the same mirror after the block exits. ADR and glossary record that an endpoint holds a resolved URL rather than resolving one.
…ads one Records why the remaining adapters get no Endpoint: without a protocol module there is no dialect to carry, and their URLs already resolve through service_url and waterdata.endpoints.
thodson-usgs
changed the title
# Conflicts: # NEWS.md # dataretrieval/ogc/policy.py
A getter now reads each date argument into a DateRange when it is called: prepare_request_args does it for every OGC date parameter, and get_ratings for its time. The value records what the caller meant -- an instant, a range with open bounds, or a duration -- and the OGC request builder only writes it. Reading moves to the dataretrieval._dates leaf; writing stays in ogc.dates. Error messages are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ect (ADR 0014) dataretrieval._intervals writes a DateRange as the ISO 8601 interval both standards define, adjusted by the endpoint's DateDialect: which parameters carry dates, date-only collections, parameters that keep a time, the open-start spelling, and whether a lone duration is sent. It replaces ogc.dates. OgcDialect holds the group as `dates`, and prepare_request_args takes it. get_ratings reaches STAC through an Endpoint with its own DateDialect and drops its duration check. Behavior change: a lone duration sent to NGWMN now raises ValueError before sending instead of failing with HTTP 400, because durations default to off, as in the standards. Live tests check open_start and durations against Water Data OGC, NGWMN, and STAC. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
OgcDialect groups its fields by the code that reads them: `dates` and `shaping` (ShapingDialect: time, numeric, sort, and extra id columns). extra_id_cols moves from a get_ogc_data argument into the shaping group, and cql2_services is renamed cql2_collections, since it lists collections. The chunk planner drops its list of Water Data date parameters: a date argument is a DateRange, never a list, so it is never split. A fitness function fails if a string a shipped dialect lists appears as a literal in ogc or _intervals. monitoring_location_id, a recorded deviation, is the one allowlisted entry. The normalizers' "value" default name is removed; every caller passes its argument's name. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review cleanups on the date-range steps: - read_date_range takes the endpoint's `durations` and refuses a duration there, when the getter is called, and words its "too many values" message from the same flag. The single_value_hint helper and parameter, and write_interval's refusal and `name`, are gone. - write_interval takes `date_only` instead of `param` and `collection`; the OGC request builder decides it, since only OGC has collections. - The Samples and Statistics getters read no parameter as a date range (`_get_args(dates=_NO_DATES)`). - _DURATION_RE and _coerce_to_list are private again; _EXTRA_ID_COLS leaves __all__; the test request builder normalizes through prepare_request_args; the fitness function drops a redundant subtraction; stale names in docs and tests are fixed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- The interval and date modules name no service: each field's observed server behavior is recorded where an adapter sets it. - A refused duration is refused "for this API": durations depend on the endpoint, and Water Data's OGC API accepts one where its STAC search does not. - read_date_range requires `name`, as every caller passes it. - Docstrings: get_water_level says a duration raises; get_ratings no longer says its time is passed through verbatim; ogc.policy describes both dialect types. - ADR 0014 notes record the two changed messages, and why cql2_collections has no live test: on 2026-10-11 a comma-joined GET returned the same rows as the CQL2 POST, in v0 and v1. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBt3Ee7eneKtdKQWq2T3YK
This branch has not been deployed
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
2 participants
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.
Summary
Proposes ADR 0014, Declare services, endpoints, and dialects as values, and lands its first steps: one
Servicedeclaration per adapter. Replaces #444, whoseOgcServicebundled a name, a URL and a dialect into one OGC-only value. The name and default URL aren't OGC-specific, and Water Data is one service with four APIs.The ADR (Proposed) defines six abstractions: service, endpoint, protocol, dialect, execution and adapter. Its rules: values are data and protocols are functions; shared code names no service; each dialect field defaults to the standard; dates are read once and written by each protocol; a protocol gets a module only once two endpoints share it. It's revised as each step lands (see its Notes) and accepted at the last step. The glossary adds Service declaration, Endpoint and Protocol, and rewords Adapter and Dialect.
Step 1: service declarations. Each adapter declares
Service(name, default_url)once, in the module that owns its URL. Every read site now takes the name from that declaration: theConfigurationclass'sadapter, theadapter=passed to retry, fan-out and pagination, and the base URL via the newconfiguration.service_url(). Water Data declares one service inwaterdata.endpoints, shared by its OGC, Samples, Statistics and STAC APIs. No behavior change.Tests
ogc/engine.py.service_urluses the default unless abase_urlsetting for that adapter overrides it.Step 2: endpoints for the OGC getters. The Water Data and NGWMN adapters each build an
Endpoint(service, url, dialect)when a call starts.get_ogc_data,_construct_api_requestsand_finalize_ogctake that one value instead of separatebase_url,dialectandadapterarguments.DEFAULT_DIALECTwas unused afterwards, so I removed it. New tests check that the engine reads the URL, dialect and settings from the endpoint, and that a call interrupted insideconfigure(base_url=...)still resumes against that mirror after the block exits. No behavior change.Step 3: no endpoints for the other adapters, by decision. The ADR now records why. An
Endpointvalue exists only where shared protocol code receives one. WQP, NLDI, NWDC, StreamStats, Samples and Statistics have no shared protocol code, so for them an endpoint would carry no dialect and would only repeat a URL lookup that already exists.Step 4: one date-range value, read when a getter is called.
dataretrieval._datesreads a date argument into aDateRange: an instant, a range with open bounds, or a duration.prepare_request_argsreads every OGC date parameter this way, andget_ratingsreads itstime. The request builders only write the value.Step 5: one interval writer for OGC and STAC, adjusted by
DateDialect.dataretrieval._intervalsreplacesogc.dates.DateDialectrecords which parameters carry dates, date-only collections, parameters that keep a time, the open-start spelling, and whether a lone duration is accepted.OgcDialect.datesholds it, andget_ratingsreaches STAC through anEndpointwith its own. Behavior change:ngwmn.get_water_level(datetime="P7D")now raisesValueErrorwhen called, instead of failing with HTTP 400 (NEWS entry added).get_ratingsdrops its own duration check for the shared one, so its message changes. New live tests checkopen_startanddurationsagainst Water Data OGC, NGWMN and STAC. All three pass as of 2026-10-11.Step 6: the OGC code names nothing a dialect declares.
OgcDialecthasdatesandshapinggroups;ShapingDialectholds the time, numeric, sort and extra-id columns.extra_id_colsmoves there from aget_ogc_dataargument.cql2_servicesis renamedcql2_collections. The chunk planner no longer lists date parameters, because aDateRangeis never split. A new fitness function fails if a string a shipped dialect lists appears as a literal inogcor_intervals;monitoring_location_id, a deviation the ADR records, is the only allowlisted entry. I checked that it fails on a planted"time".Steps 4–6 went through simplify and the two-axis review, and the findings are fixed in the last two commits.
Found while writing live tests: on 2026-10-11, a comma-joined GET on
monitoring-locationsandcombined-metadatareturned the same rows as the CQL2 POST, in both v0 and v1. That is the behaviorcql2_collectionsexists to work around, so the field may no longer be needed. I left it unchanged; removing it is a separate change.Merge order
Steps 4–6 build on #442 and #443, which are merged into this branch, so their changes show in this diff until they land. Once they merge, I'll rebase this branch onto
mainand those commits will drop out.Next step
Accept ADR 0014: reconcile the record with what was built, mark it Accepted, and update the architecture overview.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HBt3Ee7eneKtdKQWq2T3YK