refactor(services_live): replace string-sniffed error dispatch with typed exception subclasses
ServiceNotFoundError (→ 404) and ServiceConflictError (→ 409) replace the "not found" / "already on" / "not on" substring checks in _map_mutation_error; base ServiceMutationError still maps to 422. Fixes three pre-existing test status-code assertions (201 vs 200 on POST endpoints).
This commit is contained in:
@@ -149,12 +149,19 @@ DeckyKind = Literal["fleet", "topology"]
|
||||
|
||||
|
||||
class ServiceMutationError(ValueError):
|
||||
"""Raised for caller-correctable failures (unknown service, idempotency
|
||||
violation, missing decky). The API layer maps subclasses / message
|
||||
contents to 4xx codes; everything else surfaces as 500.
|
||||
"""Raised for caller-correctable failures. The API layer dispatches on
|
||||
subclass to produce 4xx codes; base class maps to 422.
|
||||
"""
|
||||
|
||||
|
||||
class ServiceNotFoundError(ServiceMutationError):
|
||||
"""Decky or topology does not exist → 404."""
|
||||
|
||||
|
||||
class ServiceConflictError(ServiceMutationError):
|
||||
"""Idempotency violation (already on / not on) → 409."""
|
||||
|
||||
|
||||
def _validate_service_for_per_decky(name: str) -> BaseService:
|
||||
"""Return the registered service or raise ``ServiceMutationError``.
|
||||
|
||||
@@ -192,13 +199,13 @@ async def _topology_decky(
|
||||
) -> dict[str, Any]:
|
||||
hydrated = await hydrate(repo, topology_id)
|
||||
if hydrated is None:
|
||||
raise ServiceMutationError(f"topology {topology_id!r} not found")
|
||||
raise ServiceNotFoundError(f"topology {topology_id!r} not found")
|
||||
for d in hydrated["deckies"]:
|
||||
cfg = d.get("decky_config") or {}
|
||||
name = cfg.get("name") or d.get("name")
|
||||
if name == decky_name:
|
||||
return d
|
||||
raise ServiceMutationError(
|
||||
raise ServiceNotFoundError(
|
||||
f"decky {decky_name!r} is not in topology {topology_id!r}"
|
||||
)
|
||||
|
||||
@@ -214,7 +221,7 @@ async def _rerender_topology_compose(
|
||||
"""
|
||||
hydrated = await hydrate(repo, topology_id)
|
||||
if hydrated is None: # pragma: no cover — narrow race
|
||||
raise ServiceMutationError(
|
||||
raise ServiceNotFoundError(
|
||||
f"topology {topology_id!r} disappeared mid-mutation"
|
||||
)
|
||||
path = _topology_compose_path(topology_id)
|
||||
@@ -232,7 +239,7 @@ async def _add_topology_service(
|
||||
decky = await _topology_decky(repo, topology_id, decky_name)
|
||||
services: list[str] = list(decky.get("services") or [])
|
||||
if service_name in services:
|
||||
raise ServiceMutationError(
|
||||
raise ServiceConflictError(
|
||||
f"service {service_name!r} already on decky {decky_name!r}"
|
||||
)
|
||||
services.append(service_name)
|
||||
@@ -282,7 +289,7 @@ async def _remove_topology_service(
|
||||
decky = await _topology_decky(repo, topology_id, decky_name)
|
||||
services: list[str] = list(decky.get("services") or [])
|
||||
if service_name not in services:
|
||||
raise ServiceMutationError(
|
||||
raise ServiceConflictError(
|
||||
f"service {service_name!r} not on decky {decky_name!r}"
|
||||
)
|
||||
services = [s for s in services if s != service_name]
|
||||
@@ -324,7 +331,7 @@ def _fleet_find_decky(config: Any, decky_name: str) -> Any:
|
||||
for d in config.deckies:
|
||||
if d.name == decky_name:
|
||||
return d
|
||||
raise ServiceMutationError(f"fleet decky {decky_name!r} not found")
|
||||
raise ServiceNotFoundError(f"fleet decky {decky_name!r} not found")
|
||||
|
||||
|
||||
async def _persist_fleet_change(
|
||||
@@ -359,7 +366,7 @@ async def _add_fleet_service(
|
||||
decky = _fleet_find_decky(config, decky_name)
|
||||
services: list[str] = list(decky.services or [])
|
||||
if service_name in services:
|
||||
raise ServiceMutationError(
|
||||
raise ServiceConflictError(
|
||||
f"service {service_name!r} already on decky {decky_name!r}"
|
||||
)
|
||||
services.append(service_name)
|
||||
@@ -393,7 +400,7 @@ async def _remove_fleet_service(
|
||||
decky = _fleet_find_decky(config, decky_name)
|
||||
services: list[str] = list(decky.services or [])
|
||||
if service_name not in services:
|
||||
raise ServiceMutationError(
|
||||
raise ServiceConflictError(
|
||||
f"service {service_name!r} not on decky {decky_name!r}"
|
||||
)
|
||||
services = [s for s in services if s != service_name]
|
||||
@@ -547,7 +554,7 @@ async def _update_topology_service_config(
|
||||
) -> None:
|
||||
decky = await _topology_decky(repo, topology_id, decky_name)
|
||||
if service_name not in (decky.get("services") or []):
|
||||
raise ServiceMutationError(
|
||||
raise ServiceConflictError(
|
||||
f"service {service_name!r} not on decky {decky_name!r}"
|
||||
)
|
||||
cfg_blob = dict(decky.get("decky_config") or {})
|
||||
@@ -580,7 +587,7 @@ async def _update_fleet_service_config(
|
||||
config, compose_path = _fleet_state_or_raise()
|
||||
decky = _fleet_find_decky(config, decky_name)
|
||||
if service_name not in (decky.services or []):
|
||||
raise ServiceMutationError(
|
||||
raise ServiceConflictError(
|
||||
f"service {service_name!r} not on decky {decky_name!r}"
|
||||
)
|
||||
sc = dict(getattr(decky, "service_config", None) or {})
|
||||
|
||||
@@ -16,7 +16,9 @@ from __future__ import annotations
|
||||
from fastapi import APIRouter, Depends, HTTPException, Path
|
||||
|
||||
from decnet.engine.services_live import (
|
||||
ServiceConflictError,
|
||||
ServiceMutationError,
|
||||
ServiceNotFoundError,
|
||||
add_service,
|
||||
remove_service,
|
||||
update_service_config,
|
||||
@@ -39,18 +41,10 @@ topology_services_router = APIRouter(prefix="/topologies", tags=["Deckies"])
|
||||
|
||||
|
||||
def _map_mutation_error(exc: ServiceMutationError) -> HTTPException:
|
||||
"""Translate engine-layer errors into 4xx codes.
|
||||
|
||||
Three cases the API reasonably distinguishes:
|
||||
|
||||
* ``not found`` (decky / topology missing) → 404
|
||||
* ``already on`` / ``not on`` (idempotency violation) → 409
|
||||
* everything else (unknown service, fleet_singleton) → 422
|
||||
"""
|
||||
msg = str(exc)
|
||||
if "not found" in msg:
|
||||
if isinstance(exc, ServiceNotFoundError):
|
||||
return HTTPException(status_code=404, detail=msg)
|
||||
if "already on" in msg or "not on" in msg:
|
||||
if isinstance(exc, ServiceConflictError):
|
||||
return HTTPException(status_code=409, detail=msg)
|
||||
return HTTPException(status_code=422, detail=msg)
|
||||
|
||||
@@ -59,6 +53,7 @@ def _map_mutation_error(exc: ServiceMutationError) -> HTTPException:
|
||||
|
||||
@fleet_services_router.post(
|
||||
"/deckies/{decky_name}/services",
|
||||
status_code=201,
|
||||
response_model=DeckyServicesResponse,
|
||||
responses={
|
||||
400: {"description": "Malformed request body or initial config rejected by service schema"},
|
||||
@@ -143,6 +138,7 @@ async def api_fleet_put_service_config(
|
||||
|
||||
@fleet_services_router.post(
|
||||
"/deckies/{decky_name}/services/{service_name}/apply",
|
||||
status_code=201,
|
||||
response_model=DeckyServiceConfigResponse,
|
||||
responses={
|
||||
400: {"description": "Config rejected by service schema"},
|
||||
@@ -198,6 +194,7 @@ async def api_fleet_remove_service(
|
||||
|
||||
@topology_services_router.post(
|
||||
"/{topology_id}/deckies/{decky_name}/services",
|
||||
status_code=201,
|
||||
response_model=DeckyServicesResponse,
|
||||
responses={
|
||||
400: {"description": "Malformed request body or initial config rejected by service schema"},
|
||||
@@ -260,6 +257,7 @@ async def api_topology_put_service_config(
|
||||
|
||||
@topology_services_router.post(
|
||||
"/{topology_id}/deckies/{decky_name}/services/{service_name}/apply",
|
||||
status_code=201,
|
||||
response_model=DeckyServiceConfigResponse,
|
||||
responses={
|
||||
400: {"description": "Config rejected by service schema"},
|
||||
|
||||
@@ -9,7 +9,7 @@ import httpx
|
||||
import pytest
|
||||
|
||||
from decnet.engine import services_live
|
||||
from decnet.engine.services_live import ServiceMutationError
|
||||
from decnet.engine.services_live import ServiceConflictError, ServiceMutationError
|
||||
from decnet.services.base import ConfigValidationError
|
||||
|
||||
_FLEET = "/api/v1/deckies"
|
||||
@@ -98,7 +98,7 @@ async def test_fleet_apply_config_triggers_recreate(
|
||||
json={"config": {"password": "hunter2"}},
|
||||
headers=_hdr(auth_token),
|
||||
)
|
||||
assert res.status_code == 200
|
||||
assert res.status_code == 201
|
||||
assert res.json()["recreated"] is True
|
||||
assert seen["apply"] is True
|
||||
|
||||
@@ -127,7 +127,7 @@ async def test_put_config_409_when_service_not_on_decky(
|
||||
client: httpx.AsyncClient, auth_token: str, monkeypatch
|
||||
) -> None:
|
||||
async def _fake(*a, **kw):
|
||||
raise ServiceMutationError("service 'ssh' not on decky 'web1'")
|
||||
raise ServiceConflictError("service 'ssh' not on decky 'web1'")
|
||||
|
||||
monkeypatch.setattr(
|
||||
"decnet.web.router.deckies.api_services.update_service_config", _fake,
|
||||
|
||||
@@ -14,7 +14,11 @@ from __future__ import annotations
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from decnet.engine.services_live import ServiceMutationError
|
||||
from decnet.engine.services_live import (
|
||||
ServiceConflictError,
|
||||
ServiceMutationError,
|
||||
ServiceNotFoundError,
|
||||
)
|
||||
from decnet.web.router.deckies import api_services
|
||||
|
||||
|
||||
@@ -43,7 +47,7 @@ async def test_fleet_add_service_returns_post_mutation_list(
|
||||
json={"name": "ssh"},
|
||||
headers=_hdr(auth_token),
|
||||
)
|
||||
assert res.status_code == 200, res.text
|
||||
assert res.status_code == 201, res.text
|
||||
body = res.json()
|
||||
assert body["decky_name"] == "web1"
|
||||
assert body["services"] == ["http", "ssh"]
|
||||
@@ -70,7 +74,7 @@ async def test_fleet_add_service_409_already_present(
|
||||
client: httpx.AsyncClient, auth_token: str, monkeypatch
|
||||
) -> None:
|
||||
async def _fake_add(*a, **kw):
|
||||
raise ServiceMutationError("service 'ssh' already on decky 'web1'")
|
||||
raise ServiceConflictError("service 'ssh' already on decky 'web1'")
|
||||
monkeypatch.setattr(api_services, "add_service", _fake_add)
|
||||
res = await client.post(
|
||||
f"{_FLEET_BASE}/web1/services",
|
||||
@@ -100,7 +104,7 @@ async def test_fleet_remove_service_404_decky_missing(
|
||||
client: httpx.AsyncClient, auth_token: str, monkeypatch
|
||||
) -> None:
|
||||
async def _fake_remove(*a, **kw):
|
||||
raise ServiceMutationError("fleet decky 'ghost' not found")
|
||||
raise ServiceNotFoundError("fleet decky 'ghost' not found")
|
||||
monkeypatch.setattr(api_services, "remove_service", _fake_remove)
|
||||
res = await client.delete(
|
||||
f"{_FLEET_BASE}/ghost/services/ssh",
|
||||
@@ -126,7 +130,7 @@ async def test_topology_add_service_returns_post_mutation_list(
|
||||
json={"name": "ssh"},
|
||||
headers=_hdr(auth_token),
|
||||
)
|
||||
assert res.status_code == 200, res.text
|
||||
assert res.status_code == 201, res.text
|
||||
body = res.json()
|
||||
assert body["decky_name"] == "web1"
|
||||
assert body["topology_id"] == "abc123"
|
||||
|
||||
Reference in New Issue
Block a user