mirror of
https://github.com/zvx-echo6/central.git
synced 2026-05-21 18:14:44 +02:00
refactor(gui): clean up flagged issues before merge
1. Make migration 015 idempotent with IF NOT EXISTS 2. Remove hardcoded cadence range from routes.py and template: - Added ge=10 constraint to AdapterConfig.cadence_s field - Removed manual 60-3600 check from routes.py POST handler - Validate cadence using AdapterConfig field metadata - Removed min/max attributes from template input 3. Move discover_adapters to its own module: - Created src/central/adapter_discovery.py - Updated supervisor.py to import from adapter_discovery - Updated routes.py to import from adapter_discovery - GUI no longer transitively imports nats or stream_manager 4. Remove dead code branch in form_descriptors.py: - Removed unreachable RegionConfig check (already handled earlier) - Improved error message for unsupported nested types 5. Updated test_adapters.py: - Changed invalid cadence test from 30 to 5 (below ge=10) - Updated assertion to check for "10" in error message Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
bff6ccffff
commit
91f1d67abd
8 changed files with 51 additions and 42 deletions
|
|
@ -3,4 +3,4 @@
|
||||||
-- Populated by supervisor when an adapter fails to start or apply config.
|
-- Populated by supervisor when an adapter fails to start or apply config.
|
||||||
|
|
||||||
ALTER TABLE config.adapters
|
ALTER TABLE config.adapters
|
||||||
ADD COLUMN last_error TEXT;
|
ADD COLUMN IF NOT EXISTS last_error TEXT;
|
||||||
|
|
|
||||||
34
src/central/adapter_discovery.py
Normal file
34
src/central/adapter_discovery.py
Normal file
|
|
@ -0,0 +1,34 @@
|
||||||
|
"""Adapter discovery utilities."""
|
||||||
|
|
||||||
|
import importlib
|
||||||
|
import logging
|
||||||
|
import pkgutil
|
||||||
|
|
||||||
|
import central.adapters
|
||||||
|
from central.adapter import SourceAdapter
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
def discover_adapters() -> dict[str, type[SourceAdapter]]:
|
||||||
|
"""Auto-discover adapter classes from central.adapters package."""
|
||||||
|
registry: dict[str, type[SourceAdapter]] = {}
|
||||||
|
for module_info in pkgutil.iter_modules(central.adapters.__path__):
|
||||||
|
try:
|
||||||
|
module = importlib.import_module(f"central.adapters.{module_info.name}")
|
||||||
|
except Exception as e:
|
||||||
|
logger.error(
|
||||||
|
"Failed to import adapter module",
|
||||||
|
extra={"module": module_info.name, "error": str(e)},
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
for attr_name in dir(module):
|
||||||
|
attr = getattr(module, attr_name)
|
||||||
|
if (
|
||||||
|
isinstance(attr, type)
|
||||||
|
and issubclass(attr, SourceAdapter)
|
||||||
|
and attr is not SourceAdapter
|
||||||
|
and hasattr(attr, "name")
|
||||||
|
):
|
||||||
|
registry[attr.name] = attr
|
||||||
|
return registry
|
||||||
|
|
@ -32,7 +32,7 @@ class AdapterConfig(BaseModel):
|
||||||
|
|
||||||
name: str = Field(description="Unique adapter identifier")
|
name: str = Field(description="Unique adapter identifier")
|
||||||
enabled: bool = Field(default=True, description="Whether adapter is active")
|
enabled: bool = Field(default=True, description="Whether adapter is active")
|
||||||
cadence_s: int = Field(description="Poll interval in seconds")
|
cadence_s: int = Field(ge=10, description="Poll interval in seconds")
|
||||||
settings: dict[str, Any] = Field(
|
settings: dict[str, Any] = Field(
|
||||||
default_factory=dict, description="Adapter-specific settings"
|
default_factory=dict, description="Adapter-specific settings"
|
||||||
)
|
)
|
||||||
|
|
|
||||||
|
|
@ -59,12 +59,12 @@ def _type_to_widget(field_name: str, field_type: type) -> str:
|
||||||
f"Field '{field_name}' has unsupported list type: list[{args[0].__name__ if args else '?'}]"
|
f"Field '{field_name}' has unsupported list type: list[{args[0].__name__ if args else '?'}]"
|
||||||
)
|
)
|
||||||
|
|
||||||
# Check if it's a BaseModel subclass (nested model)
|
# Check if it's a BaseModel subclass (nested model other than RegionConfig)
|
||||||
if isinstance(field_type, type) and issubclass(field_type, BaseModel):
|
if isinstance(field_type, type) and issubclass(field_type, BaseModel):
|
||||||
if field_type is RegionConfig:
|
|
||||||
return "region"
|
|
||||||
raise NotImplementedError(
|
raise NotImplementedError(
|
||||||
f"Field '{field_name}' has unsupported nested type: {field_type.__name__}"
|
f"Field '{field_name}' has unsupported nested type: {field_type.__name__}. "
|
||||||
|
f"If a second nested type beyond RegionConfig is needed, "
|
||||||
|
f"refactor describe_fields to recurse over nested models."
|
||||||
)
|
)
|
||||||
|
|
||||||
raise NotImplementedError(
|
raise NotImplementedError(
|
||||||
|
|
|
||||||
|
|
@ -48,7 +48,7 @@ from functools import cache
|
||||||
|
|
||||||
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.supervisor import discover_adapters
|
from central.adapter_discovery import discover_adapters
|
||||||
from pydantic import ValidationError
|
from pydantic import ValidationError
|
||||||
|
|
||||||
@cache
|
@cache
|
||||||
|
|
@ -1384,11 +1384,13 @@ async def adapters_edit_submit(
|
||||||
"cadence_s": cadence_s_str,
|
"cadence_s": cadence_s_str,
|
||||||
}
|
}
|
||||||
|
|
||||||
# Validate cadence_s
|
# Validate cadence_s using AdapterConfig field constraint (ge=10)
|
||||||
try:
|
try:
|
||||||
cadence_s = int(cadence_s_str)
|
cadence_s = int(cadence_s_str)
|
||||||
if cadence_s < 60 or cadence_s > 3600:
|
from central.config_models import AdapterConfig
|
||||||
errors["cadence_s"] = "Cadence must be between 60 and 3600 seconds"
|
min_cadence = AdapterConfig.model_fields["cadence_s"].metadata[0].ge
|
||||||
|
if cadence_s < min_cadence:
|
||||||
|
errors["cadence_s"] = f"Input should be greater than or equal to {min_cadence}"
|
||||||
except ValueError:
|
except ValueError:
|
||||||
errors["cadence_s"] = "Cadence must be a valid integer"
|
errors["cadence_s"] = "Cadence must be a valid integer"
|
||||||
cadence_s = 0
|
cadence_s = 0
|
||||||
|
|
|
||||||
|
|
@ -39,7 +39,7 @@
|
||||||
<label for="cadence_s">Cadence (seconds)</label>
|
<label for="cadence_s">Cadence (seconds)</label>
|
||||||
<input type="number" id="cadence_s" name="cadence_s"
|
<input type="number" id="cadence_s" name="cadence_s"
|
||||||
value="{{ form_data.cadence_s if form_data else adapter.cadence_s }}"
|
value="{{ form_data.cadence_s if form_data else adapter.cadence_s }}"
|
||||||
min="60" max="3600" required>
|
required>
|
||||||
{% if errors and errors.cadence_s %}
|
{% if errors and errors.cadence_s %}
|
||||||
<small style="color: var(--pico-color-red-500);">{{ errors.cadence_s }}</small>
|
<small style="color: var(--pico-color-red-500);">{{ errors.cadence_s }}</small>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
|
|
||||||
|
|
@ -13,41 +13,14 @@ from typing import Any
|
||||||
import nats
|
import nats
|
||||||
from nats.js import JetStreamContext
|
from nats.js import JetStreamContext
|
||||||
|
|
||||||
import importlib
|
|
||||||
import pkgutil
|
|
||||||
|
|
||||||
from central.adapter import SourceAdapter
|
from central.adapter import SourceAdapter
|
||||||
|
from central.adapter_discovery import discover_adapters
|
||||||
from central.cloudevents_wire import wrap_event
|
from central.cloudevents_wire import wrap_event
|
||||||
from central.config_models import AdapterConfig
|
from central.config_models import AdapterConfig
|
||||||
from central.config_source import ConfigSource, DbConfigSource
|
from central.config_source import ConfigSource, DbConfigSource
|
||||||
from central.config_store import ConfigStore
|
from central.config_store import ConfigStore
|
||||||
from central.bootstrap_config import get_settings
|
from central.bootstrap_config import get_settings
|
||||||
from central.stream_manager import StreamManager
|
from central.stream_manager import StreamManager
|
||||||
import central.adapters
|
|
||||||
|
|
||||||
def discover_adapters() -> dict[str, type[SourceAdapter]]:
|
|
||||||
"""Auto-discover adapter classes from central.adapters package."""
|
|
||||||
registry: dict[str, type[SourceAdapter]] = {}
|
|
||||||
for module_info in pkgutil.iter_modules(central.adapters.__path__):
|
|
||||||
try:
|
|
||||||
module = importlib.import_module(f"central.adapters.{module_info.name}")
|
|
||||||
except Exception as e:
|
|
||||||
logger.error(
|
|
||||||
"Failed to import adapter module",
|
|
||||||
extra={"module": module_info.name, "error": str(e)},
|
|
||||||
)
|
|
||||||
continue
|
|
||||||
for attr_name in dir(module):
|
|
||||||
attr = getattr(module, attr_name)
|
|
||||||
if (
|
|
||||||
isinstance(attr, type)
|
|
||||||
and issubclass(attr, SourceAdapter)
|
|
||||||
and attr is not SourceAdapter
|
|
||||||
and hasattr(attr, "name")
|
|
||||||
):
|
|
||||||
registry[attr.name] = attr
|
|
||||||
return registry
|
|
||||||
|
|
||||||
CURSOR_DB_PATH = Path("/var/lib/central/cursors.db")
|
CURSOR_DB_PATH = Path("/var/lib/central/cursors.db")
|
||||||
|
|
||||||
# Stream subject mappings
|
# Stream subject mappings
|
||||||
|
|
|
||||||
|
|
@ -189,7 +189,7 @@ class TestAdaptersEditSubmit:
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_adapters_edit_invalid_cadence_shows_error(self):
|
async def test_adapters_edit_invalid_cadence_shows_error(self):
|
||||||
"""POST /adapters/nws with cadence_s=30 shows error, no DB update."""
|
"""POST /adapters/nws with cadence_s=5 shows error, no DB update."""
|
||||||
from central.gui.routes import adapters_edit_submit
|
from central.gui.routes import adapters_edit_submit
|
||||||
|
|
||||||
mock_request = MagicMock()
|
mock_request = MagicMock()
|
||||||
|
|
@ -199,7 +199,7 @@ class TestAdaptersEditSubmit:
|
||||||
mock_form = MagicMock()
|
mock_form = MagicMock()
|
||||||
mock_form.get.side_effect = lambda k, d="": {
|
mock_form.get.side_effect = lambda k, d="": {
|
||||||
"csrf_token": "test_csrf_token",
|
"csrf_token": "test_csrf_token",
|
||||||
"cadence_s": "30",
|
"cadence_s": "5",
|
||||||
"contact_email": "test@example.com",
|
"contact_email": "test@example.com",
|
||||||
"region_north": "49.0",
|
"region_north": "49.0",
|
||||||
"region_south": "24.0",
|
"region_south": "24.0",
|
||||||
|
|
@ -241,7 +241,7 @@ class TestAdaptersEditSubmit:
|
||||||
call_args = mock_templates.TemplateResponse.call_args
|
call_args = mock_templates.TemplateResponse.call_args
|
||||||
context = call_args.kwargs.get("context", call_args[1].get("context"))
|
context = call_args.kwargs.get("context", call_args[1].get("context"))
|
||||||
assert "cadence_s" in context["errors"]
|
assert "cadence_s" in context["errors"]
|
||||||
assert "60" in context["errors"]["cadence_s"] or "3600" in context["errors"]["cadence_s"]
|
assert "10" in context["errors"]["cadence_s"]
|
||||||
|
|
||||||
|
|
||||||
class TestAdaptersAudit:
|
class TestAdaptersAudit:
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue