feat(types): allow extra fields on DeviceStatus - #60
Conversation
DeviceStatus had no extra policy, so pydantic silently dropped unknown keys. The registry merges heartbeat payloads as raw dicts (registry.update_status), so deployment-specific runtime state already reaches status in the device record; the typed path (DeviceDriver.status(), registration) was the only place it disappeared, without an error. Adopts the same policy DeviceIdentity has carried since the beginning, so the two halves of a device record behave consistently. Declared-field validation is unaffected (busy_score bounds still raise), and no existing device changes shape. Motivating case: a network-side location provider writing an egocentric awareness object onto nodes. Discussed as part of the halo proposal; this carries no halo-specific vocabulary in core.
b556c27 to
5ead22a
Compare
kavya-chennoju
left a comment
kavya-chennoju
left a comment
There was a problem hiding this comment.
Deep multi-agent review at head 5ead22a. Five lenses fired (correctness, error handling, security, API compat, test coverage); two candidates raised and both were killed in adversarial verification — nothing blocking. Two checks worth recording: reverting the model_config line makes test_extra_fields_allowed and test_extra_fields_roundtrip fail, so the new tests genuinely pin the fix; and model_dump(exclude_none=True) was executed under the pinned pydantic (2.12.5) and preserves non-None extras, closing the one coverage question raised. One non-blocking scope note inline about the typed registration path.
| halo={"zone": "aisle-3", "count": 3}, | ||
| ) | ||
| """ | ||
| model_config = {"extra": "allow"} |
There was a problem hiding this comment.
One layer out, the typed registration path still drops these. The registry service re-validates against its own DeviceStatus (device_connect_server/registry/service/main.py:290, no extra policy), so RegisterParams(**payload["params"]) strips extras at both registration entry points (main.py:363 and the pull path at main.py:661) before the record is written to etcd. Nothing regresses: the base branch's dict-status path (device.py:389-393) already sent extras into that strip. And the change is still real, extras now reach where-predicate contexts (device.py:1419) and D2D presence (discovery.py:97). But a driver author following the new docstring example will expect halo in the registry record, and it silently won't be there, which is the same no-error drop this PR removes at the edge. Consider mirroring extra="allow" on the server-side model in a follow-up, or noting the registry boundary in this docstring.
2 participants
What
One line:
model_config = {"extra": "allow"}onDeviceStatus, matching the policyDeviceIdentityalready has.Why
DeviceStatusdeclared noextrapolicy, so pydantic's default silently dropped unknown keys. That makes the two halves of a device record behave differently: identity accepts device-specific metadata, status does not.It also puts the typed and untyped paths out of step.
DeviceRegistry.update_status()merges heartbeat payloads into the record as raw dicts with no schema validation, so deployment-specific runtime state written over the heartbeat subject already lands instatusand is readable by agents. The only place it vanished was the typed path: a driver returning it fromDeviceDriver.status(), or an explicitDeviceStatuspassed at registration, wherestatus.model_dump(exclude_none=True)(device.py) never saw the field. No error, no log line, just a missing key.The motivating case is an authoritative network-side location provider writing an egocentric awareness object ("halo": the tracked entities around a node) onto device records. This PR carries none of that vocabulary into core, and takes no position on the naming or on selector-side exposure. It only makes the extension supported instead of accidental.
Scope
busy_score=1.5still raises, covered by a new test.model_dump(mode="json")and back.Tests
Three tests added to
TestDeviceStatusinpackages/device-connect-edge/tests/test_types.py: extras accepted and readable, extras surviving a dump/reload roundtrip, and a guard that declared-field validation still applies.test_integration.pyare Zenoh broker connection failures at fixture setup (tcp/localhost:7447, needs the Docker compose stack); they reproduce identically on unmodifiedmainin the same environment.