mirror of
https://github.com/zvx-echo6/central.git
synced 2026-05-21 18:14:44 +02:00
feat(2-G.5): preview_for_settings framework hook + NWIS opt-in
Adds an optional async hook on SourceAdapter so any adapter can surface a
settings-driven preview on its /adapters/<name> edit page. The framework
renders the result generically as a table — no adapter-name branches in
GUI templates or route code.
Framework changes:
- src/central/adapter.py: new async preview_for_settings(self, settings)
on the base class, default returns None. Adapters opt in by overriding;
non-overriding adapters render unchanged.
- src/central/gui/routes.py: GET /adapters/{name} instantiates the adapter
with a no-op _PreviewConfigStore stub and a /dev/null cursor path (GUI
has no live ConfigStore), constructs settings_obj via the schema, and
calls preview_for_settings inside a try/except. Result lands in template
context as preview_rows / preview_error.
- src/central/gui/templates/_adapter_preview.html: new partial. Generic
table with columns derived from the first dict's keys; error banner
mirrors the existing last_error article style.
- src/central/gui/templates/adapters_edit.html: one-line include between
the Region fieldset and Save/Cancel.
NWIS opt-in:
- New NWIS_MONITORING_LOCATIONS_URL constant and _PREVIEW_LIMIT cap of 50.
- preview_for_settings returns None when region is None, otherwise one-shot
fetches monitoring-locations within the bbox via a fresh aiohttp session.
Must work even when adapter is not started -- the GUI process never calls
startup(). Returns list[dict] with the contract column order: site_id,
name, site_type, state. Errors propagate so the framework can render the
operator-visible banner.
- HTTP call factored into _fetch_preview_text so tests mock cleanly.
Tests (7 new):
- tests/test_preview_hook.py: default returns None; partial renders list
with correct headers/rows/count; partial renders error banner; partial
renders empty when both context values are None.
- tests/test_nwis.py adds TestNWISPreview: returns None without region,
returns rows with correct column order, propagates HTTP errors.
Verification:
- 457/457 full suite green (was 450; +7 new tests).
- Live /adapters/nwis preview returns 50 rows with the contract keys
against the current production Iowa bbox.
- /adapters/eonet preview_for_settings returns None via base default --
proves framework is duck-typed, no NWIS-specific code in framework.
This commit is contained in:
parent
1f0e2a091e
commit
ead6ef8ce1
7 changed files with 319 additions and 1 deletions
|
|
@ -78,3 +78,12 @@ class SourceAdapter(ABC):
|
||||||
async def shutdown(self) -> None:
|
async def shutdown(self) -> None:
|
||||||
"""Optional lifecycle hook called on graceful shutdown."""
|
"""Optional lifecycle hook called on graceful shutdown."""
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
async def preview_for_settings(self, settings: BaseModel) -> list[dict] | None:
|
||||||
|
"""Optional. Override to surface a settings-driven preview on the edit page.
|
||||||
|
|
||||||
|
Return list[dict] (framework renders as a generic table; columns come from
|
||||||
|
the first dict's keys, in insertion order). Return None to skip preview.
|
||||||
|
Raise to surface an error banner — framework catches at the route boundary.
|
||||||
|
"""
|
||||||
|
return None
|
||||||
|
|
|
||||||
|
|
@ -28,6 +28,12 @@ logger = logging.getLogger(__name__)
|
||||||
NWIS_LATEST_CONTINUOUS_URL = (
|
NWIS_LATEST_CONTINUOUS_URL = (
|
||||||
"https://api.waterdata.usgs.gov/ogcapi/v0/collections/latest-continuous/items"
|
"https://api.waterdata.usgs.gov/ogcapi/v0/collections/latest-continuous/items"
|
||||||
)
|
)
|
||||||
|
NWIS_MONITORING_LOCATIONS_URL = (
|
||||||
|
"https://api.waterdata.usgs.gov/ogcapi/v0/collections/monitoring-locations/items"
|
||||||
|
)
|
||||||
|
# Per-render cap for the settings-driven preview (PR G.5). Keep small so the
|
||||||
|
# /adapters/<name> edit page renders quickly.
|
||||||
|
_PREVIEW_LIMIT = 50
|
||||||
|
|
||||||
# Single source of truth for the parameter-code default. Operators tune via
|
# Single source of truth for the parameter-code default. Operators tune via
|
||||||
# NWISSettings.parameter_codes; do NOT duplicate this list elsewhere
|
# NWISSettings.parameter_codes; do NOT duplicate this list elsewhere
|
||||||
|
|
@ -375,3 +381,55 @@ class NWISAdapter(SourceAdapter):
|
||||||
geo=Geo(centroid=centroid),
|
geo=Geo(centroid=centroid),
|
||||||
data=data,
|
data=data,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
async def _fetch_preview_text(self, url: str) -> str:
|
||||||
|
"""One-shot GET for the preview render.
|
||||||
|
|
||||||
|
Uses a fresh aiohttp session — preview must work even when the adapter
|
||||||
|
isn't started (the GUI process never calls startup()). Factored out so
|
||||||
|
tests can mock the HTTP call without touching aiohttp internals.
|
||||||
|
"""
|
||||||
|
async with aiohttp.ClientSession(
|
||||||
|
timeout=aiohttp.ClientTimeout(total=15),
|
||||||
|
) as session:
|
||||||
|
async with session.get(
|
||||||
|
url, headers={"User-Agent": "Central/0.4"}
|
||||||
|
) as resp:
|
||||||
|
resp.raise_for_status()
|
||||||
|
return await resp.text()
|
||||||
|
|
||||||
|
async def preview_for_settings(self, settings: NWISSettings) -> list[dict] | None:
|
||||||
|
"""Surface monitoring-locations inside the configured bbox.
|
||||||
|
|
||||||
|
Returns up to _PREVIEW_LIMIT rows from the monitoring-locations
|
||||||
|
collection. Returns None if region is unset (no useful preview).
|
||||||
|
Raises on HTTP / JSON / shape failure — framework catches at the route.
|
||||||
|
"""
|
||||||
|
if settings.region is None:
|
||||||
|
return None
|
||||||
|
|
||||||
|
params = {
|
||||||
|
"bbox": (
|
||||||
|
f"{settings.region.west},{settings.region.south},"
|
||||||
|
f"{settings.region.east},{settings.region.north}"
|
||||||
|
),
|
||||||
|
"limit": str(_PREVIEW_LIMIT),
|
||||||
|
}
|
||||||
|
url = f"{NWIS_MONITORING_LOCATIONS_URL}?{urlencode(params)}"
|
||||||
|
|
||||||
|
text = await self._fetch_preview_text(url)
|
||||||
|
page = json.loads(text)
|
||||||
|
features = page.get("features") or []
|
||||||
|
|
||||||
|
rows: list[dict] = []
|
||||||
|
for feat in features:
|
||||||
|
props = feat.get("properties") or {}
|
||||||
|
rows.append(
|
||||||
|
{
|
||||||
|
"site_id": feat.get("id"),
|
||||||
|
"name": props.get("monitoring_location_name"),
|
||||||
|
"site_type": props.get("site_type_code"),
|
||||||
|
"state": props.get("state_name"),
|
||||||
|
}
|
||||||
|
)
|
||||||
|
return rows
|
||||||
|
|
|
||||||
|
|
@ -46,6 +46,9 @@ from central.gui.audit import (
|
||||||
)
|
)
|
||||||
from functools import cache
|
from functools import cache
|
||||||
|
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from central.config_models import AdapterConfig
|
||||||
from central.gui.db import get_pool
|
from central.gui.db import get_pool
|
||||||
from central.gui.form_descriptors import describe_fields, FieldDescriptor
|
from central.gui.form_descriptors import describe_fields, FieldDescriptor
|
||||||
from central.adapter_discovery import discover_adapters
|
from central.adapter_discovery import discover_adapters
|
||||||
|
|
@ -55,13 +58,23 @@ from pydantic import ValidationError
|
||||||
@cache
|
@cache
|
||||||
def _adapter_classes() -> dict:
|
def _adapter_classes() -> dict:
|
||||||
"""Cached adapter class discovery.
|
"""Cached adapter class discovery.
|
||||||
|
|
||||||
GUI is a separate process from supervisor; walks pkgutil itself.
|
GUI is a separate process from supervisor; walks pkgutil itself.
|
||||||
Python's import cache makes subsequent calls free.
|
Python's import cache makes subsequent calls free.
|
||||||
"""
|
"""
|
||||||
return discover_adapters()
|
return discover_adapters()
|
||||||
|
|
||||||
|
|
||||||
|
class _PreviewConfigStore:
|
||||||
|
"""No-op stand-in passed to adapter __init__ when calling preview_for_settings.
|
||||||
|
|
||||||
|
preview_for_settings implementations must create their own one-shot HTTP
|
||||||
|
session and must not depend on config_store / cursor_db state — the GUI
|
||||||
|
process has no live ConfigStore (the supervisor owns the real one)."""
|
||||||
|
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
router = APIRouter()
|
router = APIRouter()
|
||||||
|
|
||||||
# Streams to display on dashboard -- derived from the registry's dashboard flag.
|
# Streams to display on dashboard -- derived from the registry's dashboard flag.
|
||||||
|
|
@ -1450,6 +1463,28 @@ async def adapters_edit_form(
|
||||||
)
|
)
|
||||||
api_key_missing = not has_key
|
api_key_missing = not has_key
|
||||||
|
|
||||||
|
# Generic settings-driven preview. Adapters opt in by overriding
|
||||||
|
# SourceAdapter.preview_for_settings; the framework is duck-typed on the
|
||||||
|
# returned list[dict] shape and never branches on adapter name.
|
||||||
|
preview_rows: list[dict] | None = None
|
||||||
|
preview_error: str | None = None
|
||||||
|
if adapter_cls is not None and hasattr(adapter_cls, "settings_schema"):
|
||||||
|
try:
|
||||||
|
settings_obj = adapter_cls.settings_schema(**settings)
|
||||||
|
preview_cfg = AdapterConfig(
|
||||||
|
name=row["name"],
|
||||||
|
enabled=row["enabled"],
|
||||||
|
cadence_s=row["cadence_s"],
|
||||||
|
settings=settings,
|
||||||
|
updated_at=row["updated_at"],
|
||||||
|
)
|
||||||
|
preview_adapter = adapter_cls(
|
||||||
|
preview_cfg, _PreviewConfigStore(), Path("/dev/null")
|
||||||
|
)
|
||||||
|
preview_rows = await preview_adapter.preview_for_settings(settings_obj)
|
||||||
|
except Exception as exc:
|
||||||
|
preview_error = f"Preview unavailable: {exc}"
|
||||||
|
|
||||||
csrf_token = request.state.csrf_token
|
csrf_token = request.state.csrf_token
|
||||||
response = templates.TemplateResponse(
|
response = templates.TemplateResponse(
|
||||||
request=request,
|
request=request,
|
||||||
|
|
@ -1466,6 +1501,8 @@ async def adapters_edit_form(
|
||||||
"tile_attribution": tile_attribution,
|
"tile_attribution": tile_attribution,
|
||||||
"api_key_missing": api_key_missing,
|
"api_key_missing": api_key_missing,
|
||||||
"requires_api_key_alias": requires_api_key_alias,
|
"requires_api_key_alias": requires_api_key_alias,
|
||||||
|
"preview_rows": preview_rows,
|
||||||
|
"preview_error": preview_error,
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
return response
|
return response
|
||||||
|
|
|
||||||
23
src/central/gui/templates/_adapter_preview.html
Normal file
23
src/central/gui/templates/_adapter_preview.html
Normal file
|
|
@ -0,0 +1,23 @@
|
||||||
|
{% if preview_error %}
|
||||||
|
<article aria-label="Preview Unavailable" style="background-color: var(--pico-del-color); margin-bottom: 1rem;">
|
||||||
|
<strong>{{ preview_error }}</strong>
|
||||||
|
</article>
|
||||||
|
{% elif preview_rows %}
|
||||||
|
<fieldset>
|
||||||
|
<legend>Preview ({{ preview_rows|length }} rows)</legend>
|
||||||
|
<table class="preview-table" role="grid">
|
||||||
|
<thead>
|
||||||
|
<tr>
|
||||||
|
{% for col in preview_rows[0].keys() %}<th>{{ col }}</th>{% endfor %}
|
||||||
|
</tr>
|
||||||
|
</thead>
|
||||||
|
<tbody>
|
||||||
|
{% for row in preview_rows %}
|
||||||
|
<tr>
|
||||||
|
{% for col in preview_rows[0].keys() %}<td>{{ row[col] }}</td>{% endfor %}
|
||||||
|
</tr>
|
||||||
|
{% endfor %}
|
||||||
|
</tbody>
|
||||||
|
</table>
|
||||||
|
</fieldset>
|
||||||
|
{% endif %}
|
||||||
|
|
@ -178,6 +178,8 @@
|
||||||
</fieldset>
|
</fieldset>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
|
||||||
|
{% include "_adapter_preview.html" %}
|
||||||
|
|
||||||
<button type="submit">Save Changes</button>
|
<button type="submit">Save Changes</button>
|
||||||
<a href="/adapters" role="button" class="outline">Cancel</a>
|
<a href="/adapters" role="button" class="outline">Cancel</a>
|
||||||
</form>
|
</form>
|
||||||
|
|
|
||||||
|
|
@ -276,3 +276,84 @@ class TestNWISAdapter:
|
||||||
|
|
||||||
assert first_pass
|
assert first_pass
|
||||||
assert second_pass == []
|
assert second_pass == []
|
||||||
|
|
||||||
|
|
||||||
|
class TestNWISPreview:
|
||||||
|
"""Preview hook (PR G.5) — exercised without starting the adapter."""
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_preview_returns_none_without_region(self, tmp_path: Path):
|
||||||
|
from central.adapters.nwis import NWISAdapter, NWISSettings
|
||||||
|
|
||||||
|
adapter = NWISAdapter(_config({}), MagicMock(), tmp_path / "cursors.db")
|
||||||
|
result = await adapter.preview_for_settings(NWISSettings())
|
||||||
|
assert result is None
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_preview_returns_rows_with_region(self, tmp_path: Path):
|
||||||
|
"""Given a bbox, preview returns one dict per monitoring-locations feature
|
||||||
|
with the contract column order: site_id, name, site_type, state."""
|
||||||
|
from central.adapters.nwis import NWISAdapter, NWISSettings
|
||||||
|
|
||||||
|
sample_response = json.dumps({
|
||||||
|
"type": "FeatureCollection",
|
||||||
|
"features": [
|
||||||
|
{
|
||||||
|
"type": "Feature",
|
||||||
|
"id": "USGS-05420500",
|
||||||
|
"geometry": {"type": "Point", "coordinates": [-90.25, 41.78]},
|
||||||
|
"properties": {
|
||||||
|
"monitoring_location_name": "MISSISSIPPI RIVER AT CLINTON, IA",
|
||||||
|
"site_type_code": "ST",
|
||||||
|
"state_name": "Iowa",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": "Feature",
|
||||||
|
"id": "USGS-05454500",
|
||||||
|
"geometry": {"type": "Point", "coordinates": [-91.65, 41.66]},
|
||||||
|
"properties": {
|
||||||
|
"monitoring_location_name": "IOWA RIVER AT IOWA CITY, IA",
|
||||||
|
"site_type_code": "ST",
|
||||||
|
"state_name": "Iowa",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
})
|
||||||
|
|
||||||
|
adapter = NWISAdapter(
|
||||||
|
_config({"region": {"west": -94.0, "south": 40.0, "east": -93.0, "north": 41.0}}),
|
||||||
|
MagicMock(),
|
||||||
|
tmp_path / "cursors.db",
|
||||||
|
)
|
||||||
|
adapter._fetch_preview_text = AsyncMock(return_value=sample_response)
|
||||||
|
settings = NWISSettings(region=adapter.region)
|
||||||
|
rows = await adapter.preview_for_settings(settings)
|
||||||
|
|
||||||
|
assert rows is not None
|
||||||
|
assert len(rows) == 2
|
||||||
|
# Column order is part of the contract — first row's keys must match exactly.
|
||||||
|
expected_keys = ["site_id", "name", "site_type", "state"]
|
||||||
|
assert list(rows[0].keys()) == expected_keys
|
||||||
|
# Row content reflects fixture data.
|
||||||
|
assert rows[0]["site_id"] == "USGS-05420500"
|
||||||
|
assert rows[0]["name"] == "MISSISSIPPI RIVER AT CLINTON, IA"
|
||||||
|
assert rows[0]["site_type"] == "ST"
|
||||||
|
assert rows[0]["state"] == "Iowa"
|
||||||
|
assert rows[1]["site_id"] == "USGS-05454500"
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_preview_propagates_http_error(self, tmp_path: Path):
|
||||||
|
"""Preview must not swallow upstream errors — the framework needs them
|
||||||
|
to render the operator-visible 'Preview unavailable: …' banner."""
|
||||||
|
from central.adapters.nwis import NWISAdapter, NWISSettings
|
||||||
|
|
||||||
|
adapter = NWISAdapter(
|
||||||
|
_config({"region": {"west": -94.0, "south": 40.0, "east": -93.0, "north": 41.0}}),
|
||||||
|
MagicMock(),
|
||||||
|
tmp_path / "cursors.db",
|
||||||
|
)
|
||||||
|
adapter._fetch_preview_text = AsyncMock(side_effect=RuntimeError("upstream 503"))
|
||||||
|
settings = NWISSettings(region=adapter.region)
|
||||||
|
with pytest.raises(RuntimeError, match="upstream 503"):
|
||||||
|
await adapter.preview_for_settings(settings)
|
||||||
|
|
|
||||||
108
tests/test_preview_hook.py
Normal file
108
tests/test_preview_hook.py
Normal file
|
|
@ -0,0 +1,108 @@
|
||||||
|
"""Tests for the SourceAdapter.preview_for_settings hook + framework rendering."""
|
||||||
|
|
||||||
|
from collections.abc import AsyncIterator
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import jinja2
|
||||||
|
import pytest
|
||||||
|
from pydantic import BaseModel
|
||||||
|
|
||||||
|
from central.adapter import SourceAdapter
|
||||||
|
from central.models import Event
|
||||||
|
|
||||||
|
TEMPLATES_DIR = Path(__file__).resolve().parents[1] / "src" / "central" / "gui" / "templates"
|
||||||
|
|
||||||
|
|
||||||
|
class _StubSettings(BaseModel):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class _StubAdapter(SourceAdapter):
|
||||||
|
"""Minimal SourceAdapter subclass that does NOT override preview_for_settings."""
|
||||||
|
|
||||||
|
name = "stub"
|
||||||
|
display_name = "Stub Adapter"
|
||||||
|
description = "Test fixture"
|
||||||
|
settings_schema = _StubSettings
|
||||||
|
default_cadence_s = 60
|
||||||
|
|
||||||
|
def __init__(self) -> None:
|
||||||
|
pass
|
||||||
|
|
||||||
|
async def poll(self) -> AsyncIterator[Event]: # pragma: no cover - never invoked
|
||||||
|
if False:
|
||||||
|
yield # type: ignore[unreachable]
|
||||||
|
return
|
||||||
|
|
||||||
|
async def apply_config(self, new_config) -> None: # pragma: no cover
|
||||||
|
pass
|
||||||
|
|
||||||
|
def subject_for(self, event: Event) -> str: # pragma: no cover
|
||||||
|
return "stub"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_default_returns_none():
|
||||||
|
"""The base SourceAdapter's preview_for_settings default returns None.
|
||||||
|
|
||||||
|
Any adapter that does not override the hook gets a no-op preview, so the
|
||||||
|
framework renders the page without a preview block (no crash, no opt-out
|
||||||
|
boilerplate required in each adapter).
|
||||||
|
"""
|
||||||
|
adapter = _StubAdapter()
|
||||||
|
result = await adapter.preview_for_settings(_StubSettings())
|
||||||
|
assert result is None
|
||||||
|
|
||||||
|
|
||||||
|
def _render_partial(**context) -> str:
|
||||||
|
env = jinja2.Environment(
|
||||||
|
loader=jinja2.FileSystemLoader(str(TEMPLATES_DIR)),
|
||||||
|
autoescape=jinja2.select_autoescape(["html"]),
|
||||||
|
)
|
||||||
|
tmpl = env.get_template("_adapter_preview.html")
|
||||||
|
return tmpl.render(**context)
|
||||||
|
|
||||||
|
|
||||||
|
def test_partial_renders_list():
|
||||||
|
"""Given list[dict] with insertion-ordered keys, partial renders a table.
|
||||||
|
|
||||||
|
Framework is duck-typed: columns come from the first dict's keys. No
|
||||||
|
adapter-name branches, no per-adapter Jinja.
|
||||||
|
"""
|
||||||
|
rows = [
|
||||||
|
{"a": "x1", "b": "y1"},
|
||||||
|
{"a": "x2", "b": "y2"},
|
||||||
|
]
|
||||||
|
out = _render_partial(preview_rows=rows, preview_error=None)
|
||||||
|
# Header row uses first dict's keys in order.
|
||||||
|
assert "<th>a</th>" in out
|
||||||
|
assert "<th>b</th>" in out
|
||||||
|
# Body rows carry every value.
|
||||||
|
for cell in ("x1", "y1", "x2", "y2"):
|
||||||
|
assert cell in out
|
||||||
|
# Row count surfaces in the legend.
|
||||||
|
assert "Preview (2 rows)" in out
|
||||||
|
# No error banner when preview_error is None.
|
||||||
|
assert "Preview Unavailable" not in out
|
||||||
|
|
||||||
|
|
||||||
|
def test_partial_renders_error():
|
||||||
|
"""When preview_error is set, partial renders the error banner and no table."""
|
||||||
|
out = _render_partial(preview_rows=None, preview_error="Preview unavailable: boom")
|
||||||
|
assert "Preview unavailable: boom" in out
|
||||||
|
# No table when an error is set.
|
||||||
|
assert "<table" not in out
|
||||||
|
assert "<th>" not in out
|
||||||
|
|
||||||
|
|
||||||
|
def test_partial_renders_nothing_when_both_none():
|
||||||
|
"""No preview_rows and no preview_error -> partial renders no preview section.
|
||||||
|
|
||||||
|
Lets adapters that don't opt in (e.g. EONET, GDACS) render normally without
|
||||||
|
any preview-related markup on the page.
|
||||||
|
"""
|
||||||
|
out = _render_partial(preview_rows=None, preview_error=None).strip()
|
||||||
|
assert "<table" not in out
|
||||||
|
assert "Preview Unavailable" not in out
|
||||||
|
# Either empty or only whitespace/newlines from the template.
|
||||||
|
assert "Preview (" not in out
|
||||||
Loading…
Add table
Add a link
Reference in a new issue