Use full match in homeassistant backup excludes #5597
Conversation
📝 WalkthroughWalkthroughThe changes focus on refining the backup exclusion mechanism for Home Assistant in the supervisor module. The modifications update the file exclusion logic in the backup process, adding new exclusion patterns like Changes
Sequence DiagramsequenceDiagram
participant Backup as Backup Process
participant Excluder as Exclusion Filter
participant File as File System
Backup->>Excluder: Check file for exclusion
Excluder->>File: Apply full_match exclusion criteria
File-->>Excluder: Return match result
Excluder-->>Backup: Determine whether to include/exclude file
The sequence diagram illustrates the updated backup exclusion process, showing how files are now checked against exclusion criteria using a more precise ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/homeassistant/test_module.py (1)
150-181: Consider adding assertions for backup content.While the test verifies that files are correctly logged as excluded, it would be valuable to also assert that excluded files are actually not present in the backup archive.
Here's how you could enhance the test:
async def test_backup_excludes( coresys: CoreSys, caplog: pytest.LogCaptureFixture, filename: str, exclude_db: bool, expect_excluded: bool, subfolder: str | None, ): """Test excludes in backup.""" parent = coresys.config.path_homeassistant if subfolder: test_path = PurePath(subfolder, filename) parent = coresys.config.path_homeassistant / subfolder parent.mkdir(parents=True) else: test_path = PurePath(filename) (parent / filename).touch() backup = Backup(coresys, coresys.config.path_backup / "test.tar", "test", None) backup.new("test", utcnow().isoformat(), BackupType.PARTIAL) async with backup.create(): with ( patch.object(HomeAssistant, "begin_backup"), patch.object(HomeAssistant, "end_backup"), caplog.at_level(logging.DEBUG, logger="supervisor.homeassistant.module"), ): await backup.store_homeassistant(exclude_database=exclude_db) assert ( f"Ignoring data/{test_path.as_posix()} because of " in caplog.text ) is expect_excluded + + # Verify file presence/absence in the backup + with tarfile.open(backup.tarfile, "r:") as tar: + file_list = tar.getnames() + backup_path = f"data/{test_path.as_posix()}" + assert (backup_path in file_list) is not expect_excluded
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
supervisor/homeassistant/module.py(2 hunks)tests/homeassistant/test_module.py(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build armv7 supervisor
- GitHub Check: Build armhf supervisor
- GitHub Check: Build aarch64 supervisor
- GitHub Check: Run tests Python 3.13.1
🔇 Additional comments (4)
tests/homeassistant/test_module.py (2)
30-32: LGTM! Good practice adding explicit encoding.Adding explicit UTF-8 encoding when opening files is a good practice to ensure consistent behavior across different platforms.
120-147: Well-structured test cases covering various backup exclusion scenarios.The parameterized test cases thoroughly cover:
- Log files with different extensions
- Database files with and without exclusion flag
- Files in backup and tts directories
- Files in
__pycache__directories.DS_Storefiles- Files in subfolders vs root directory
This provides good coverage of the backup exclusion logic.
supervisor/homeassistant/module.py (2)
63-71: LGTM! Improved backup exclusion patterns.The updated patterns provide better coverage:
- Using
**/__pycache__/*ensures all pycache directories are excluded- Adding
**/.DS_Storehelps clean up macOS metadata files- Adding
*.logcatches all log files- Explicit paths for backup and tts directories prevent accidental exclusions
425-425: Good use offull_matchfor precise path matching.Using
full_matchinstead ofmatchensures that paths are matched exactly, preventing accidental exclusions of similarly named paths. The "data/" prefix correctly aligns with the backup archive structure.
| "backups/*.tar", | ||
| "tmp_backups/*.tar", |
There was a problem hiding this comment.
Technically those two are really only required for Core installations, since in Supervisor we store backups elsewhere. However, they don't really hurt. And maybe if someone just copied the /config directory, it might even be helpful 🤷♂️
Proposed change
Use
full_matchfor paths to exclude from Home Assistant backups. This prevents accidental exclusion of things like user config subfolders that happened to be calledttsorbackups.Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit
New Features
Tests
Bug Fixes