Skip to content

perf: skip is_package() network call for already-synced Drive files#449

Merged
mandarons merged 4 commits into
mainfrom
copilot/fix-drive-synchronization-duration
May 21, 2026
Merged

perf: skip is_package() network call for already-synced Drive files#449
mandarons merged 4 commits into
mainfrom
copilot/fix-drive-synchronization-duration

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 20, 2026

  • Identify root cause: is_package() in drive_parallel_download.py makes a network request for every file before checking local existence
  • Fix collect_file_for_download in drive_parallel_download.py to check local file existence FIRST before making any network request
  • Fix process_file in sync_drive.py (legacy function) with same optimization, eliminating duplicate package post-download code via if/elif/else structure
  • Improve comments to clearly explain the side-effect of package_exists() and the directory-implies-package assumption
  • Add test test_collect_file_for_download_package_outdated to cover the new early-return path for outdated package directories
  • Identify CI coverage failure: drive_file_existence.py lines 68-69 lost coverage because the new code only calls package_exists() when os.path.isdir() is True (old code called it for first-time package downloads with non-existent paths)
  • Add test_package_exists_none_item and test_package_exists_non_existent_path to restore 100% coverage of drive_file_existence.py
  • All 92 drive sync tests pass; all changed files at 100% coverage; linter clean

Copilot AI and others added 2 commits May 20, 2026 15:09
Drive sync was making one HTTP network request per file via is_package()
before checking whether the file already existed locally. For thousands
of already-synced files this caused significant unnecessary latency.

Now collect_file_for_download() and process_file() check local existence
first (no network call):
- If local file exists and is up-to-date → skip immediately
- If local directory exists and package is up-to-date → skip immediately
- If local directory exists but package is outdated → mark as package for
  re-download (avoids is_package() call by inferring type from filesystem)
- Only call is_package() when neither local file nor directory exists"

Agent-Logs-Url: https://github.com/mandarons/icloud-docker/sessions/628ead08-5a77-4a33-b669-8899f8477db8

Co-authored-by: mandarons <50469173+mandarons@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix long duration for drive synchronization without transfers perf: skip is_package() network call for already-synced Drive files May 20, 2026
Copilot AI requested a review from mandarons May 20, 2026 15:12
@mandarons mandarons marked this pull request as ready for review May 20, 2026 16:13
Copilot AI review requested due to automatic review settings May 20, 2026 16:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves iCloud Drive sync performance by avoiding the expensive is_package() HTTP request for items that are already present locally and up-to-date. It updates both the parallel download path and the legacy process_file() path to check local existence first, and adds targeted tests to keep coverage at 100%.

Changes:

  • Reordered Drive sync decision logic to check os.path.isfile() / os.path.isdir() and local freshness (file_exists() / package_exists()) before calling is_package().
  • Added an early-return download-task path for “outdated local package directory” cases to re-download packages without an extra is_package() network call.
  • Added/expanded unit tests to cover package_exists() edge cases and the new early-return behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/drive_parallel_download.py Skips is_package() for already-synced files/packages by checking local existence first; adds fast re-download scheduling for outdated package dirs.
src/sync_drive.py Applies the same optimization to legacy process_file() and simplifies the control flow with an explicit if/elif/else structure.
tests/test_sync_drive.py Adds tests covering package_exists() falsey cases and the new “outdated package dir” early-return path in collect_file_for_download.

@mandarons mandarons merged commit 98c5c5f into main May 21, 2026
6 checks passed
@mandarons mandarons deleted the copilot/fix-drive-synchronization-duration branch May 21, 2026 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Long Duration for Drive Synchronization without transfers

3 participants