One typed home for the config snapshot - #60
Merged
Conversation
jfmlima
marked this pull request as ready for review
July 30, 2026 11:58
jfmlima
force-pushed
the
refactor/config-snapshot
branch
from
July 30, 2026 12:35
e49c67b to
344df10
Compare
Both containers build AsyncShellyRPCClient; the synchronous client had no production callers and was kept alive only by its own test.
get_bulk_status and BulkStatusRequest had no production callers; the API and CLI read device status one device at a time through CheckDeviceStatusUseCase.
CheckDeviceStatusUseCase never read it: device status always carries firmware information. The CLI flag could not even express a false value (is_flag with default=True), the API query parameter changed nothing, and the web client never passed it.
The backup file format was defined implicitly by whatever export_bulk_config happened to build, and every consumer re-derived it by string key: the backup use case probed captured entries for a restorable payload, the restore strategies read "type"/"success"/"config"/"code" off raw dicts, and the Gen1 strategy dug the legacy settings out of the snapshot itself. DeviceSnapshot now owns that shape, with to_dict/from_dict as the only place the stored key names live. CaptureDeviceConfig produces it for both the bulk export and a stored backup, so a backup takes one device status fetch instead of two, and the capture strategies hand back typed entries. Restore parses the stored snapshot once and hands ComponentSnapshot to its strategies. Devices in a bulk export are captured concurrently. Gathering with return_exceptions keeps a failing device from leaving its siblings running against real hardware with nobody awaiting them.
The three shelly-component commands were one shape repeated, differing only in the action name and the strings a failure reports. Applying config to several devices now runs them concurrently, the way the gateway already runs bulk actions. Each device's own components are still configured one at a time, and results stay in device order.
A component entry that is not a mapping was skipped while parsing, so a restore that asked for that key reported it as absent from the backup. It is corrupt, not absent, and reporting it as missing contradicts the rule that a caller never silently gets a smaller selection than they asked for. It now parses as a failed capture, and the export pairs IPs with captures strictly.
Overlapping targets ("-t 10.0.0.5 -t 10.0.0.0/24") expand to the same IP more
than once, and handling devices concurrently turned that into two overlapping
requests against one device, which is exactly what the per-device sequencing
avoids. Repeated IPs now collapse, in the order first given.
A caller who passed an IP twice gets one result for it rather than two
identical ones, and the export metadata counts the devices actually captured.
A Gen1 restore replays the raw /settings and nothing else, so a snapshot without them aborts every restore. The mapped component configs left behind still looked like a healthy capture, so such a backup was stored and only revealed itself as unusable at restore time. This overrides the capture docstring's claim that the mapped configs alone make a Gen1 backup valid: they are Gen2-shaped and no Gen1 endpoint accepts them. Storing one promises a recovery it cannot deliver.
Applying config across devices used a plain gather, so one device raising propagated immediately while its siblings carried on issuing SetConfig to real hardware with nobody awaiting them. That is the same hazard the capture path already guarded against, and it matters more here because these calls write. Both paths now share one helper that runs every device to completion before surfacing the first failure.
jfmlima
force-pushed
the
refactor/config-snapshot
branch
from
July 30, 2026 13:33
344df10 to
e0cfa03
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Give the device configuration snapshot one typed definition that capture writes and restore reads, and tidy the bulk operations around it.
Context
The backup file format was defined implicitly by whatever
export_bulk_confighappened to build, and every consumer rediscovered it by string key: the backup use case probed captured entries for a restorable payload, both restore strategies readtype/success/config/codeoff raw dicts, and the Gen1 strategy dug the raw settings out of the snapshot itself.DeviceSnapshotnow owns that shape, withto_dictandfrom_dictas the only place the stored key names live, and one capture use case produces it for both the bulk export and a stored backup. A backup consequently reads a device's status once rather than twice.The stored format is the risk here, since snapshots are persisted encrypted and served to the web. It is pinned three ways: a round trip test over realistic Gen1 and Gen2 snapshots; the old and new capture code run against identical mocked gateways over 256 scenarios (512 captures, zero mismatches) and the old and new restore over 252 scenarios, comparing both the result and the exact sequence of gateway calls; and a backup taken from a real Gen4 device through the API under docker compose, which came out byte identical to one taken by the previous code.
Dropping
include_updatesis the one user visible change.CheckDeviceStatusUseCasenever read it, the CLI flag could not express a false value (is_flagwithdefault=Trueand no secondary), and the web client never passed it. An old client still sending?include_updates=falsegets an identical response, since Litestar ignores the unknown parameter. It is kept as its own commit so it can be dropped without taking the rest.Notes
Two things here change behaviour rather than shape.
A Gen1 snapshot captured without the raw
/settingsis now refused rather than stored. A Gen1 restore replays those settings and nothing else, so such a backup aborts every restore, and storing one promises a recovery it cannot deliver. That overrides the capture docstring's claim that the mapped configs alone still make a Gen1 backup valid, so it is a separate commit and easy to drop. It is also the one path not exercised against hardware, since there is no Gen1 device on the network.Repeated IPs in a bulk operation now collapse. Overlapping targets expand to the same device twice, and handling devices concurrently turned that into two overlapping writes to a single device.