Downloader updated to skip unchanged files #3182
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes the Predbat update/download process by skipping downloads of files that haven't changed, comparing local file SHA1 hashes against remote GitHub file hashes. When a match is found, the local file is copied to the staging location instead of being re-downloaded from GitHub.
Key Changes
- SHA-based download optimization that skips unchanged files by comparing local and remote file hashes
- Two comprehensive test cases validating skip behavior for both full-match and mixed scenarios
- Enhanced user feedback showing which files were skipped during updates
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/predbat/download.py | Implements SHA comparison logic to skip downloading unchanged files, copying local versions to staging instead |
| apps/predbat/tests/test_download.py | Adds two test cases: one verifying skip behavior when SHAs match, another testing mixed scenarios with both matching and non-matching files |
| with open(local_filepath, "rb") as src: | ||
| with open(download_filepath, "wb") as dst: |
There was a problem hiding this comment.
File mode inconsistency detected. The copy operation uses binary mode ("rb"/"wb") while download_predbat_file_from_github uses text mode ("w"). This inconsistency could cause different line endings on Windows (CRLF vs LF) between downloaded files and copied files, potentially leading to SHA mismatches on subsequent checks. For consistency, either both should use binary mode, or both should use text mode. Recommend using binary mode for both since the SHA computation in compute_file_sha1 operates on binary data.
| with open(local_filepath, "rb") as src: | |
| with open(download_filepath, "wb") as dst: | |
| with open(local_filepath, "r") as src: | |
| with open(download_filepath, "w") as dst: |
|
|
||
| # Download all files | ||
| # Download all files (skip if local file has matching SHA) | ||
| downloaded_files = [] |
There was a problem hiding this comment.
The variable name downloaded_files is misleading because it includes both downloaded files and skipped files. Files that are skipped (due to matching SHA) are added to this list at line 246, even though they weren't actually downloaded from GitHub. Consider renaming this to processed_files or staged_files to better reflect that it includes all files prepared for installation, regardless of whether they were downloaded or copied from local.
No description provided.