Restore persistent directory when a download fails#5378
Open
frenck wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes update/install rollback behavior so a repository’s persistent_directory is always moved back into place, even when the download step fails, preventing user data from being stranded in /tmp and lost on reboot.
Changes:
- Wraps repository download steps in
try/finallyand restores the persistent directory infinally. - Adds a regression test covering a failed update download with an existing persistent directory.
- Adds an API-usage snapshot for the new test.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| custom_components/hacs/repositories/base.py | Ensures persistent directory restoration runs on both success and failure paths during install/update. |
| tests/repositories/test_update_repository.py | Adds regression test verifying persistent user data survives a failed download. |
| tests/snapshots/api-usage/tests/repositories/test_update_repositorytest-update-repository-entity-download-failure-keeps-persistent-directory.json | Records expected API usage for the new regression test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Proposed change
When downloading an update for an installed repository, the persistent directory (
persistent_directoryinhacs.json) is moved out of the install directory into/tmpbefore the backup of the old install is created. That backup therefore never contains the persistent directory.The restore of the persistent directory only happened on the success path. On a failed download, the error path raised before reaching it: the backup restore brought back the old install without the persistent data, which stayed stranded in
/tmpand was gone after a reboot.This change wraps the download steps in
try/finallyand moves the persistent directory restore into thefinallyblock, so it is always moved back in place. This also covers download paths that raise directly (like "No content to download") instead of going through the validation error branch.Added a test that upgrades an installed integration with a persistent directory present, injects a download failure, and asserts the user data is still there afterwards. The test fails on the previous code.
Type of change
Checklist