mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-19 07:33:18 +02:00
Say why a preset's values are unavailable, not just that they are
The settings panel collapsed four causes into one message -- "the picked
preset's own values could not be read" -- with no indication of what to do
about it.
The overwhelmingly common cause has an obvious fix, and it isn't an edge
case: an install pulls its sidecar as SIDECAR_TAG:-latest regardless of
which Bambuddy channel it is on, so a current Bambuddy talking to a
sidecar that predates POST /profiles/resolve is the normal state, not a
misconfiguration. Those users would have seen an amber warning on every
slice with nothing pointing at the sidecar image.
resolve_profile now returns ResolvedProfile(values, reason) instead of
None for everything, the route passes the reason through, and the panel
picks its message from it:
sidecar_outdated -> name the fix: update the sidecar image
sidecar_unavailable -> the sidecar did not answer
not_configured -> no sidecar is configured
preset_unresolved -> the previous generic wording
A request that fails outright maps to sidecar_unavailable, since a
backend we cannot reach and a sidecar that will not answer are the same
thing from the dialog.
Every variant still ends with "anything you don't change still uses the
preset" -- that reassurance is the point of the notice, and it is true
whichever way the lookup failed.
Tests pin the distinction rather than just the happy path: a 404 and a
500 must produce different reasons, and each panel case asserts both that
its own message appears and that the "update the sidecar image" line does
not leak into the others.
This commit is contained in:
@@ -563,40 +563,45 @@ async def get_preset_values(
|
||||
OrcaSlicer's published tree instead, which can disagree with what actually
|
||||
slices; showing numbers from it would be confidently wrong.
|
||||
|
||||
Returns ``{"resolved": false, "values": {}}`` rather than an error whenever
|
||||
the values can't be obtained (sidecar offline, too old for the endpoint, or
|
||||
slicing not configured). The panel then falls back to schema defaults and
|
||||
tells the user the values are indicative, which is a far better outcome
|
||||
than a modal that won't open.
|
||||
Returns ``{"resolved": false, "values": {}, "reason": "..."}`` rather than
|
||||
an error whenever the values can't be obtained. ``reason`` is what makes
|
||||
the fallback actionable: a Bambuddy install pulls its sidecar as
|
||||
``SIDECAR_TAG:-latest`` regardless of its own release channel, so the
|
||||
overwhelmingly common cause is a sidecar older than the endpoint — which
|
||||
the user fixes by pulling a newer image, if we tell them that instead of
|
||||
"could not read the values".
|
||||
"""
|
||||
if slot != "process":
|
||||
raise HTTPException(status_code=400, detail="Only the 'process' slot is supported")
|
||||
|
||||
ref = PresetRef(source=source, id=id)
|
||||
|
||||
def unresolved(reason: str) -> dict:
|
||||
return {"resolved": False, "values": {}, "reason": reason}
|
||||
|
||||
try:
|
||||
profile_json = await resolve_preset_ref(db, current_user, ref, slot)
|
||||
except HTTPException:
|
||||
# A preset the caller can't resolve is not a reason to break the panel;
|
||||
# the slice itself will report it properly if they go ahead.
|
||||
logger.info("Could not resolve %s preset %s for value lookup", slot, id)
|
||||
return {"resolved": False, "values": {}}
|
||||
return unresolved("preset_unresolved")
|
||||
|
||||
api_url = await _resolve_slicer_api_url(db)
|
||||
if not api_url:
|
||||
return {"resolved": False, "values": {}}
|
||||
return unresolved("not_configured")
|
||||
|
||||
service = SlicerApiService(api_url)
|
||||
try:
|
||||
values = await service.resolve_profile(profile_json, "process")
|
||||
resolved = await service.resolve_profile(profile_json, "process")
|
||||
except SlicerApiUnavailableError:
|
||||
return {"resolved": False, "values": {}}
|
||||
return unresolved("sidecar_unavailable")
|
||||
finally:
|
||||
await service.close()
|
||||
|
||||
if values is None:
|
||||
return {"resolved": False, "values": {}}
|
||||
return {"resolved": True, "values": values}
|
||||
if resolved.values is None:
|
||||
return unresolved(resolved.reason)
|
||||
return {"resolved": True, "values": resolved.values, "reason": "ok"}
|
||||
|
||||
|
||||
@router.get("/presets", response_model=UnifiedPresetsResponse)
|
||||
|
||||
@@ -54,6 +54,18 @@ class SlicerTimeoutError(SlicerApiError):
|
||||
"""
|
||||
|
||||
|
||||
class ResolvedProfile(NamedTuple):
|
||||
"""A preset's effective values, or why they are unavailable.
|
||||
|
||||
``reason`` is one of ``ok`` / ``sidecar_outdated`` / ``sidecar_unavailable``
|
||||
/ ``preset_unresolved``. It exists so the UI can say something actionable
|
||||
instead of one generic "could not read the values" for four causes.
|
||||
"""
|
||||
|
||||
values: dict | None
|
||||
reason: str
|
||||
|
||||
|
||||
class SliceResult(NamedTuple):
|
||||
"""Result of a slice operation."""
|
||||
|
||||
@@ -310,7 +322,7 @@ class SlicerApiService:
|
||||
raise SlicerApiUnavailableError(f"Slicer sidecar /health returned {response.status_code}")
|
||||
return response.json()
|
||||
|
||||
async def resolve_profile(self, profile_json: str, category: str) -> dict | None:
|
||||
async def resolve_profile(self, profile_json: str, category: str) -> "ResolvedProfile":
|
||||
"""POST /profiles/resolve — flatten a preset's ``inherits:`` chain.
|
||||
|
||||
Returns the effective key/value map the slicer would actually use, so
|
||||
@@ -325,15 +337,19 @@ class SlicerApiService:
|
||||
the one baked into the running sidecar image — values from it would look
|
||||
authoritative and could quietly disagree with what gets sliced.
|
||||
|
||||
Returns ``None`` when the sidecar is too old to have the endpoint, so
|
||||
callers can degrade to schema defaults instead of failing the modal.
|
||||
Genuine transport failures still raise.
|
||||
Returns a :class:`ResolvedProfile` whose ``reason`` distinguishes *why*
|
||||
values are missing. That matters more than it looks: the common case in
|
||||
practice is a sidecar older than this endpoint, because a Bambuddy
|
||||
install pulls ``SIDECAR_TAG:-latest`` independently of its own release
|
||||
channel. "Could not read the values" sends that user hunting; "your
|
||||
sidecar image is older than this feature" is a one-line fix. Genuine
|
||||
transport failures still raise.
|
||||
"""
|
||||
try:
|
||||
payload = json.loads(profile_json)
|
||||
except json.JSONDecodeError:
|
||||
logger.warning("Cannot resolve %s preset: content is not valid JSON", category)
|
||||
return None
|
||||
return ResolvedProfile(None, "preset_unresolved")
|
||||
|
||||
try:
|
||||
response = await self._client.post(
|
||||
@@ -345,21 +361,25 @@ class SlicerApiService:
|
||||
raise SlicerApiUnavailableError(f"Slicer sidecar unreachable: {exc}") from exc
|
||||
|
||||
if response.status_code == 404:
|
||||
# Sidecar predates the endpoint. Not an error — the caller shows
|
||||
# schema defaults and says so.
|
||||
# Sidecar predates the endpoint. Not an error, and specifically not
|
||||
# the same as a broken one — this is the case that has a fix the
|
||||
# user can act on.
|
||||
logger.info("Slicer sidecar has no /profiles/resolve; falling back to schema defaults")
|
||||
return None
|
||||
return ResolvedProfile(None, "sidecar_outdated")
|
||||
if response.status_code >= 400:
|
||||
logger.warning(
|
||||
"Slicer sidecar /profiles/resolve returned %s: %s",
|
||||
response.status_code,
|
||||
_format_sidecar_error(response),
|
||||
)
|
||||
return None
|
||||
return ResolvedProfile(None, "sidecar_unavailable")
|
||||
|
||||
body = response.json()
|
||||
resolved = body.get("profile") if isinstance(body, dict) else None
|
||||
return resolved if isinstance(resolved, dict) else None
|
||||
if not isinstance(resolved, dict):
|
||||
logger.warning("Slicer sidecar /profiles/resolve returned no profile object")
|
||||
return ResolvedProfile(None, "sidecar_unavailable")
|
||||
return ResolvedProfile(resolved, "ok")
|
||||
|
||||
async def list_bundled_profiles(self) -> dict:
|
||||
"""GET /profiles/bundled — return the slicer's stock profiles by slot.
|
||||
|
||||
@@ -34,22 +34,28 @@ class TestResolveProfile:
|
||||
return httpx.Response(200, json={"profile": {"line_width": "0.42", "wall_loops": "2"}})
|
||||
|
||||
service = _service(handler)
|
||||
assert await service.resolve_profile(PROCESS_STUB, "process") == {
|
||||
"line_width": "0.42",
|
||||
"wall_loops": "2",
|
||||
}
|
||||
result = await service.resolve_profile(PROCESS_STUB, "process")
|
||||
assert result.values == {"line_width": "0.42", "wall_loops": "2"}
|
||||
assert result.reason == "ok"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_sidecar_without_the_endpoint_is_not_an_error(self):
|
||||
# Older images 404 here. The panel degrades to schema defaults and says
|
||||
# so; failing would make the modal unusable against an old sidecar.
|
||||
async def test_a_sidecar_without_the_endpoint_is_reported_as_outdated(self):
|
||||
# Older images 404 here. This is the dominant case in practice -- an
|
||||
# install pulls SIDECAR_TAG:-latest regardless of its own release
|
||||
# channel -- and it is the one with a fix the user can act on, so it
|
||||
# must not be flattened into the generic failure.
|
||||
service = _service(lambda request: httpx.Response(404, json={"message": "Not Found"}))
|
||||
assert await service.resolve_profile(PROCESS_STUB, "process") is None
|
||||
result = await service.resolve_profile(PROCESS_STUB, "process")
|
||||
assert result.values is None
|
||||
assert result.reason == "sidecar_outdated"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_sidecar_error_degrades_rather_than_raising(self):
|
||||
async def test_a_sidecar_error_is_not_reported_as_outdated(self):
|
||||
# A broken sidecar and an old one call for different advice.
|
||||
service = _service(lambda request: httpx.Response(500, json={"message": "boom"}))
|
||||
assert await service.resolve_profile(PROCESS_STUB, "process") is None
|
||||
result = await service.resolve_profile(PROCESS_STUB, "process")
|
||||
assert result.values is None
|
||||
assert result.reason == "sidecar_unavailable"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unreachable_sidecar_still_raises(self):
|
||||
@@ -62,15 +68,19 @@ class TestResolveProfile:
|
||||
await _service(handler).resolve_profile(PROCESS_STUB, "process")
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unparseable_preset_content_returns_none(self):
|
||||
async def test_unparseable_preset_content_blames_the_preset(self):
|
||||
service = _service(lambda request: httpx.Response(200, json={"profile": {}}))
|
||||
assert await service.resolve_profile("not json", "process") is None
|
||||
result = await service.resolve_profile("not json", "process")
|
||||
assert result.values is None
|
||||
assert result.reason == "preset_unresolved"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_response_without_a_profile_object_returns_none(self):
|
||||
async def test_a_response_without_a_profile_object_returns_no_values(self):
|
||||
# Guards against reading a differently-shaped body as if it were values.
|
||||
service = _service(lambda request: httpx.Response(200, json={"ok": True}))
|
||||
assert await service.resolve_profile(PROCESS_STUB, "process") is None
|
||||
result = await service.resolve_profile(PROCESS_STUB, "process")
|
||||
assert result.values is None
|
||||
assert result.reason == "sidecar_unavailable"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_already_flat_preset_round_trips(self):
|
||||
@@ -81,7 +91,7 @@ class TestResolveProfile:
|
||||
assert "inherits" not in body["profile"]
|
||||
return httpx.Response(200, json={"profile": json.loads(flat)})
|
||||
|
||||
assert await _service(handler).resolve_profile(flat, "process") == {
|
||||
assert (await _service(handler).resolve_profile(flat, "process")).values == {
|
||||
"line_width": "0.45",
|
||||
"type": "process",
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user