mirror of
https://github.com/zvx-echo6/central.git
synced 2026-05-21 18:14:44 +02:00
Merge feature/2-g5-preview-hook (PR G.5: preview_for_settings framework hook)
feat(2-G.5): preview_for_settings framework hook + NWIS opt-in
This commit is contained in:
commit
93b412fa22
7 changed files with 344 additions and 1 deletions
|
|
@ -78,3 +78,23 @@ class SourceAdapter(ABC):
|
|||
async def shutdown(self) -> None:
|
||||
"""Optional lifecycle hook called on graceful shutdown."""
|
||||
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.
|
||||
|
||||
Contract:
|
||||
- Preview is a pure function of `settings`. Do NOT access
|
||||
self._config_store or cursor_db state — the framework may instantiate
|
||||
adapters with a stub config_store solely to call this method.
|
||||
- Network preview implementations must open their own short-lived
|
||||
aiohttp session (the adapter's polling session may not exist; the GUI
|
||||
process never calls startup()).
|
||||
- Return None when preview is not meaningful (e.g., required settings
|
||||
like region are unset). Return [] explicitly if the query ran and
|
||||
matched zero rows — the framework renders that distinctly from None.
|
||||
"""
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -28,6 +28,12 @@ logger = logging.getLogger(__name__)
|
|||
NWIS_LATEST_CONTINUOUS_URL = (
|
||||
"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
|
||||
# NWISSettings.parameter_codes; do NOT duplicate this list elsewhere
|
||||
|
|
@ -375,3 +381,55 @@ class NWISAdapter(SourceAdapter):
|
|||
geo=Geo(centroid=centroid),
|
||||
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 pathlib import Path
|
||||
|
||||
from central.config_models import AdapterConfig
|
||||
from central.gui.db import get_pool
|
||||
from central.gui.form_descriptors import describe_fields, FieldDescriptor
|
||||
from central.adapter_discovery import discover_adapters
|
||||
|
|
@ -55,13 +58,23 @@ from pydantic import ValidationError
|
|||
@cache
|
||||
def _adapter_classes() -> dict:
|
||||
"""Cached adapter class discovery.
|
||||
|
||||
|
||||
GUI is a separate process from supervisor; walks pkgutil itself.
|
||||
Python's import cache makes subsequent calls free.
|
||||
"""
|
||||
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()
|
||||
|
||||
# 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
|
||||
|
||||
# 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
|
||||
response = templates.TemplateResponse(
|
||||
request=request,
|
||||
|
|
@ -1466,6 +1501,8 @@ async def adapters_edit_form(
|
|||
"tile_attribution": tile_attribution,
|
||||
"api_key_missing": api_key_missing,
|
||||
"requires_api_key_alias": requires_api_key_alias,
|
||||
"preview_rows": preview_rows,
|
||||
"preview_error": preview_error,
|
||||
},
|
||||
)
|
||||
return response
|
||||
|
|
|
|||
25
src/central/gui/templates/_adapter_preview.html
Normal file
25
src/central/gui/templates/_adapter_preview.html
Normal file
|
|
@ -0,0 +1,25 @@
|
|||
{% 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 is not none %}
|
||||
<fieldset>
|
||||
<legend>Preview ({{ preview_rows|length }} rows)</legend>
|
||||
{% if preview_rows %}
|
||||
<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>
|
||||
{% endif %}
|
||||
</fieldset>
|
||||
{% endif %}
|
||||
|
|
@ -178,6 +178,8 @@
|
|||
</fieldset>
|
||||
{% endif %}
|
||||
|
||||
{% include "_adapter_preview.html" %}
|
||||
|
||||
<button type="submit">Save Changes</button>
|
||||
<a href="/adapters" role="button" class="outline">Cancel</a>
|
||||
</form>
|
||||
|
|
|
|||
|
|
@ -276,3 +276,84 @@ class TestNWISAdapter:
|
|||
|
||||
assert first_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)
|
||||
|
|
|
|||
120
tests/test_preview_hook.py
Normal file
120
tests/test_preview_hook.py
Normal file
|
|
@ -0,0 +1,120 @@
|
|||
"""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
|
||||
|
||||
|
||||
def test_partial_renders_empty_list():
|
||||
"""Empty list -> legend with (0 rows), no table.
|
||||
|
||||
Distinct from None (which renders nothing at all). Lets adapters signal
|
||||
'query ran, matched zero rows' separately from 'preview not meaningful'.
|
||||
"""
|
||||
out = _render_partial(preview_rows=[], preview_error=None)
|
||||
assert "Preview (0 rows)" in out
|
||||
assert "<table" not in out
|
||||
assert "<th>" not in out
|
||||
Loading…
Add table
Add a link
Reference in a new issue