feat(status): say how old the printer's report is, not just what it said - #7
Conversation
The retained status is mostly a memory. The D30 is only reachable while it is being printed to, so the agent remembers what it last heard and republishes it rather than going quiet (labelfab#12). That is the right trade -- the alternative is waking a sleeping printer to keep a status page tidy -- but it means "media ok" on the Machines page is usually a reading from the last job, and until now it looked exactly like a reading from a minute ago. That is the same mistake the tri-state media_ok exists to prevent, one layer along: this module already refuses to render silence as health, and rendering a three-day-old observation as a current one is the same claim by another route. labelfab now publishes device_seen_at, so the age is shown: CONNECTED fw 2.1.2 · 100% (4.17V) · media ok · Q223P4C31420105 · seen 6h ago It annotates the reading rather than overruling it -- a printer that had tape six hours ago is far likelier to still have tape than to have anything else -- so the status mapping is untouched. No stale-to-WARNING rule; that would be inventing policy. _ago never raises. It runs in a Django worker on a payload produced by another process, so a surprise there is a 500 on the Machines page rather than a red test: an absent, empty or unparseable timestamp renders no age at all, which also keeps an agent too old to publish the field looking exactly as it did before. Two details that are easy to get wrong and were verified rather than assumed. The package supports Python 3.9, whose fromisoformat rejects the trailing Z that pydantic emits for UTC -- confirmed against a real 3.9.25 interpreter, where it raises ValueError -- so the offset is normalised by hand. And a naive timestamp is read as UTC rather than local, which would otherwise shift the age by whole hours in a way nobody would notice. classify_status takes an injectable now, so the ages are asserted against a fixed instant instead of wall time.
|
Both nits taken, and the pairing dependency is documented.
On the stale-LWT pairing: correct that this side cannot detect it, and correct that the fix belongs upstream. The producer PR now re-arms the will on every reconnect (labelfab@09c6487), which bounds it by the reconnect interval rather than the process lifetime — but that does not close the case you described, because the broker holds the will from CONNECT and nothing can change it for a live session. Neither of your two suggestions closes it either: What actually bounds it is recovery, not prevention: the unit is 49 tests on 3.12, 37 on a real 3.9.25, ruff clean. |
…nside it (#8) Follow-up to #7, which merged before these review points were addressed. Same three changes, rebased onto the merged main. ## The window the age cannot cover Review raised a case this module cannot defend against, and it is worth writing down rather than leaving to be rediscovered. The agent's MQTT will is fixed when its connection opens and the broker holds it, so a fault learned *during* that session never reaches it. Agent connects while the tape is fine → a print an hour later finds the tape gone and correctly publishes `media_ok: false` → agent is killed → the broker publishes the older, *healthier* reading over the newer one, and this page renders `DISCONNECTED` with a stale-healthy detail line. Nothing here can detect it: retained MQTT v3.1.1 messages carry no publish timestamp. So the README note says where the recovery actually comes from rather than pretending to a fix — the agent restarts (`Restart=always`, `RestartSec=5`), seeds from its spool, which *does* hold the fault, and republishes. Seconds wide, not indefinite. Worth being precise about what does **not** close it, since both were suggested: `on_disconnect` fires with no connection to publish on, and a SIGKILL runs no callback at all. Re-arming the will on reconnect (done in sengine-cloud/labelfab#13) bounds it by the reconnect interval rather than the process lifetime, but cannot touch a live session. The recovery path is now pinned by a test upstream rather than left as an argument. ## Two nits `_ago`'s `seen` is annotated `object`, deliberately not `str | None` — it comes out of `json.loads` on a payload this process did not produce and can be any JSON type, so a narrow annotation would move the lie into the signature rather than remove it. Under 10 seconds renders `seen just now` instead of `seen 0s ago`. That is the common case, since the page is usually looked at right after printing something. ## Verified 49 tests on 3.12, 37 on a real 3.9.25 in Docker, ruff clean.
1 participant
Consumer half of sengine-cloud/labelfab#12 — pairs with sengine-cloud/labelfab#13.
Why
The retained status is mostly a memory. The D30 is only reachable while it is being printed to, so the agent remembers what it last heard and republishes it rather than going quiet. That is the right trade — the alternative is waking a sleeping printer to keep a status page tidy — but it means
media okon the Machines page is usually a reading from the last job, and until now it looked exactly like a reading from a minute ago.This module already refuses to render silence as health. Rendering a three-day-old observation as a current one is the same claim by another route.
What it looks like
The age comes from
device_seen_at, which labelfab#13 adds to the retained status. It annotates the reading rather than overruling it — a printer that had tape six hours ago is far likelier to still have tape than to have anything else — so the status→member mapping is completely untouched. No stale→WARNING rule: that would be inventing policy this issue did not ask for._agonever raises. It runs inside a Django worker on a payload produced by another process, so a surprise there is a 500 on the Machines page rather than a red test. An absent, empty or unparseable timestamp renders no age at all — which also means an agent too old to publish the field looks exactly as it does today.Two details verified, not assumed
This package supports Python 3.9, whose
fromisoformatrejects the trailingZthat pydantic emits for UTC. Confirmed against a real interpreter rather than from memory:So the offset is normalised by hand. CI only runs 3.12, where this would have passed silently and failed in production. The whole status suite was also run under 3.9.25 in Docker — 36 passed.
A naive timestamp is read as UTC rather than local time, which would otherwise shift the age by whole hours in a way nobody would spot.
Verified
48 tests pass on 3.12, 36 status tests pass on a real 3.9.25, ruff clean.
classify_statustakes an injectablenow, so the ages are asserted against a fixed instant instead of wall time; the driver keeps calling it positionally and is unchanged.