Stop a firmware download when the service is stopping #63

Merged
Claude merged 5 commits from fix/stop-a-download-on-shutdown into feat/firmware-updates 2026-09-19 20:09:26 +00:00
Collaborator

A fetch runs in a thread — the poll's, or the one the control API answers on — and serve only cancels the coroutine waiting on it. The thread carried on reading, so a container asked to stop sat there until the image finished arriving or Docker killed it. On a Pi losing power that is a shutdown cut off part way through a write.

firmware.FETCH_STOP is what the thread watches. ForgejoReleases.download now reads a chunk at a time instead of copyfileobj and gives up between chunks, and _check stops before opening a connection for the next asset. An interrupted fetch raises FetchStoppedError, which store already cleans up after: the .part goes, no image.json is written, and the next check fetches the image again. refresh and refresh_now release REFRESH_LOCK on that path as on any other.

Measured with a 20 MB image trickling in over 16 seconds: stopping took 15.8 s before and 0.049 s after — one chunk. A read that hangs outright is still bounded by REQUEST_TIMEOUT rather than by this.

Tested with six tests that all fail without the change: the fetch giving up between chunks, a check that begins while stopping fetching nothing, the cache left with no usable entry, a later check fetching the image again, the lock free afterwards, and a stop while a fetch is in flight getting the thread back inside a second. Each drives the real ForgejoReleases through a fake opener that hands the image out in small pieces, so none of them depends on scheduling luck. A conftest fixture gives every test its own FETCH_STOP.

CCS-UHA-11.

A fetch runs in a thread — the poll's, or the one the control API answers on — and `serve` only cancels the coroutine waiting on it. The thread carried on reading, so a container asked to stop sat there until the image finished arriving or Docker killed it. On a Pi losing power that is a shutdown cut off part way through a write. `firmware.FETCH_STOP` is what the thread watches. `ForgejoReleases.download` now reads a chunk at a time instead of `copyfileobj` and gives up between chunks, and `_check` stops before opening a connection for the next asset. An interrupted fetch raises `FetchStoppedError`, which `store` already cleans up after: the `.part` goes, no `image.json` is written, and the next check fetches the image again. `refresh` and `refresh_now` release `REFRESH_LOCK` on that path as on any other. Measured with a 20 MB image trickling in over 16 seconds: stopping took 15.8 s before and 0.049 s after — one chunk. A read that hangs outright is still bounded by `REQUEST_TIMEOUT` rather than by this. Tested with six tests that all fail without the change: the fetch giving up between chunks, a check that begins while stopping fetching nothing, the cache left with no usable entry, a later check fetching the image again, the lock free afterwards, and a stop while a fetch is in flight getting the thread back inside a second. Each drives the real `ForgejoReleases` through a fake opener that hands the image out in small pieces, so none of them depends on scheduling luck. A conftest fixture gives every test its own `FETCH_STOP`. CCS-UHA-11.
Stop a firmware download when the service is stopping
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m47s
Lint, type check and test / release (pull_request) Has been skipped
2f6494bf71
Claude left a comment

Three points, none of them blocking the mechanism itself — the stop reaches the thread and the cache is left clean.

Three points, none of them blocking the mechanism itself — the stop reaches the thread and the cache is left clean.
@ -514,0 +686,4 @@
def crawl() -> None:
started.set()
time.sleep(CRAWL)
Author
Collaborator

Only test here that turns on elapsed time: it proves the stop by finishing inside 1s where the full crawl needs 6s. It cannot pass falsely, but a stalled box flakes it to fail. If you want it exact, have between block on a threading.Event the test releases a chunk at a time instead of sleeping CRAWL.

Only test here that turns on elapsed time: it proves the stop by finishing inside 1s where the full crawl needs 6s. It cannot pass falsely, but a stalled box flakes it to fail. If you want it exact, have `between` block on a `threading.Event` the test releases a chunk at a time instead of sleeping `CRAWL`.
Author
Collaborator

A stopped download is reported as a fetch failure. store returns None on FetchStoppedError, so the asset lands in Check.failed, and button.py turns that into HomeAssistantError("The check could not fetch node_lighting-....bin"). Verified: a check stopped mid-download returns reached=True, failed=('node_lighting-2026.09.4-firmware.bin',). Skip the append when FETCH_STOP.is_set(), and assert on failed in one of the new tests — nothing covers it today.

A stopped download is reported as a fetch failure. `store` returns `None` on `FetchStoppedError`, so the asset lands in `Check.failed`, and `button.py` turns that into `HomeAssistantError("The check could not fetch node_lighting-....bin")`. Verified: a check stopped mid-download returns `reached=True, failed=('node_lighting-2026.09.4-firmware.bin',)`. Skip the append when `FETCH_STOP.is_set()`, and assert on `failed` in one of the new tests — nothing covers it today.
@ -38,2 +38,4 @@
"""
settings.firmware_cache.mkdir(parents=True, exist_ok=True)
# Cleared on the way in so a stop cannot outlive the run that asked for it.
firmware.FETCH_STOP.clear()
Author
Collaborator

clear() here is unreachable in production and untested. serve is called once per process (main -> run -> serve), and the full suite passes with this line deleted because the conftest fixture gives each test its own event. Drop it, or keep it with a test for the in-process restart it is guarding.

`clear()` here is unreachable in production and untested. `serve` is called once per process (`main` -> `run` -> `serve`), and the full suite passes with this line deleted because the conftest fixture gives each test its own event. Drop it, or keep it with a test for the in-process restart it is guarding.
Take the review: a stop is not a fetch that failed
Some checks failed
Lint, type check and test / release (pull_request) Has been cancelled
Lint, type check and test / hassfest (pull_request) Has been cancelled
Lint, type check and test / quality (pull_request) Has been cancelled
0f5f19af60
Take the review: drop a clear the service never reaches twice
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 20s
Lint, type check and test / release (pull_request) Has been cancelled
Lint, type check and test / quality (pull_request) Has been cancelled
329c1f8e4d
Take the review: hand the stopped fetch its chunks by hand
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 28s
Lint, type check and test / release (pull_request) Has been cancelled
Lint, type check and test / quality (pull_request) Has been cancelled
2de8d32020
Merge remote-tracking branch 'origin/feat/firmware-updates' into fix/stop-a-download-on-shutdown
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 12s
Lint, type check and test / quality (pull_request) Successful in 1m39s
Lint, type check and test / release (pull_request) Has been skipped
9c9fc54853
Claude left a comment

All three taken, nothing further. The handshake test now passes in 10ms with no clock in it, and still fails when the chunk check or serve's FETCH_STOP.set() is removed. The merge of the base is clean: the three-dot diff is still only conftest, test_firmware_cache, firmware.py and service.py.

All three taken, nothing further. The handshake test now passes in 10ms with no clock in it, and still fails when the chunk check or `serve`'s `FETCH_STOP.set()` is removed. The merge of the base is clean: the three-dot diff is still only conftest, test_firmware_cache, firmware.py and service.py.
Claude merged commit adcfebc210 into feat/firmware-updates 2026-09-19 20:09:26 +00:00
Claude deleted branch fix/stop-a-download-on-shutdown 2026-09-19 20:09:26 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/CampervanHomeAssistant!63
No description provided.