Remove I/O in event loop for Home Assistant Core backup #5648
Conversation
agners
added
the
refactor
The Home Assistant Core backup still contains some I/O in the event loop. Move all I/O into the executor.
agners
force-pushed
the
remove-io-fron-evevnt-loop-in-ha-core-backup
branch
from
0d15b09 to
556de56
Compare
📝 WalkthroughWalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant HA as HomeAssistant
participant EF as _is_excluded_by_filter
participant WT as _write_tarfile
participant WJ as write_json_file
C->>HA: backup(tar_file, exclude_database)
HA->>HA: Log backup start
HA->>HA: Build exclusions list
HA->>EF: Evaluate if file is excluded
HA->>WT: Call _write_tarfile(metadata)
WT->>WJ: Write JSON file using metadata
HA->>HA: Log backup complete
HA->>C: Return backup result
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
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)
supervisor/homeassistant/module.py (1)
412-449: Consider adding progress reporting for large backups.While the current implementation effectively moves I/O out of the event loop, for large backup operations, it might be beneficial to add progress reporting.
def _write_tarfile(metadata: dict[str, Any]) -> None: """Write tarfile.""" with TemporaryDirectory(dir=self.sys_config.path_tmp) as temp: temp_path = Path(temp) + total_files = sum(1 for _ in self.sys_config.path_homeassistant.rglob('*') if _.is_file()) + processed_files = 0 # Store local configs/state try: write_json_file(temp_path.joinpath("homeassistant.json"), metadata) except ConfigurationFileError as err: raise HomeAssistantError( f"Can't save meta for Home Assistant Core: {err!s}", _LOGGER.error, ) from err try: with tar_file as backup: # Backup metadata backup.add(temp, arcname=".") + def _progress_callback(path: Path) -> None: + """Report backup progress.""" + nonlocal processed_files + processed_files += 1 + _LOGGER.debug("Backup progress: %d/%d files", processed_files, total_files) # Backup data atomic_contents_add( backup, self.sys_config.path_homeassistant, file_filter=_is_excluded_by_filter, + progress_callback=_progress_callback, arcname="data", )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supervisor/homeassistant/module.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run the Supervisor
- GitHub Check: Run tests Python 3.13.2
🔇 Additional comments (4)
supervisor/homeassistant/module.py (4)
396-396: LGTM! Minor docstring improvement.The docstring update from "config/ directory" to "config/directory" improves clarity.
398-411: Well-structured refactoring of exclusion filter logic.The extraction of filter logic into
_is_excluded_by_filterfunction improves code organization and readability. The function effectively:
- Encapsulates the exclusion logic
- Provides helpful debug logging
- Uses clear variable names
412-443: Effective removal of I/O operations from event loop.The refactoring successfully moves I/O operations into the executor by:
- Encapsulating tarfile operations in
_write_tarfile- Passing metadata explicitly instead of accessing instance state
- Using
atomic_contents_addfor safe file operationsThis aligns perfectly with the PR's objective of removing I/O from the event loop.
445-451: Improved backup process flow with better error handling.The changes enhance the backup process by:
- Using clear logging for start and completion
- Properly handling cleanup in the
finallyblock- Moving I/O operations to executor via
sys_run_in_executorThis ensures the event loop remains responsive during backup operations.
Co-authored-by: Mike Degatano <[email protected]>
Proposed change
The Home Assistant Core backup still contains some I/O in the event loop. Move all I/O into the executor.
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit