Plugin system with example Immich plugin #1302
Open
Conversation
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
This reverts commit c82491d.
The test_handle_session_error_during_download now uses a cassette with an actual session error response instead of mocking PyiCloudAPIResponseException. This works because the error handling code in session.py (before commit c82491d) properly parses error responses and raises exceptions. Changes: - Created listing_photos_session_error_download.yml cassette with 401 "Invalid global session" response - Removed all mocks from test_handle_session_error_during_download - Test now relies entirely on cassette for session error simulation - Demonstrates that cassettes can trigger PyiCloudAPIResponseException without mocks This is a cleaner approach that tests the actual error handling path rather than bypassing it with mocks. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…tests - Converted 4 session error tests from using mocks to using VCR cassettes - test_handle_session_error_during_download: uses cassette with 401 response during download - test_handle_session_error_during_photo_iteration: uses cassette with 401 response during photo listing - test_handle_session_error_during_download_name_id7: uses cassette for name-id7 file match policy - test_handle_session_error_during_photo_iteration_name_id7: uses cassette for name-id7 with listing error All tests properly trigger PyiCloudAPIResponseException from HTTP responses in cassettes without needing to mock the exception directly. This proves that cassettes can simulate session errors when the error handling code is active. Note: One test (download_name_id7) needs cassette completion for the second download attempt after re-authentication. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Since MAX_RETRIES is set to 0 and retry logic was removed, the time.sleep mocks and sleep count assertions are no longer needed. The tests now only verify that re-authentication happens (by counting 'Authenticating...' messages) and that the session error message appears in the output. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove mock.patch.object for send_verification_code in test_2sa_flow_failed_send_code - Create dedicated VCR cassette (2sa_flow_failed_send_code.yml) with failed response - Cassette simulates service unavailable (success:false) response - Remove unnecessary mock import from test file This demonstrates that mocks can be replaced with more realistic VCR cassettes that test the full HTTP request/response handling stack. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Use consistent error response format with errorTitle, errorMessage, and errorCode - Match the structure used in other Apple API error responses - Remove incorrectly added 'status' field from headers section - Update content-length to match new response size The response now matches Apple's actual API format as seen in other captured cassettes like 2sa_flow_invalid_code.yml. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
… mocks - Created listing_albums_error.yml cassette with 500 status and API error response - Removed PhotoLibrary._fetch_folders mock and authenticate mock - Test now relies on cassette to trigger PyiCloudAPIResponseException - Removed unused PhotoLibrary import 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The session.py code has two paths for error handling:
1. HTTP errors (non-OK status codes)
2. API errors in JSON payload with 200 status
The test_handle_albums_error was originally mocking an API error, so the
cassette should return HTTP 200 with error in the JSON payload, not HTTP 500.
Updated cassette response to: {"success":false,"error":"Api Error","errorCode":"100"}
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <[email protected]>
- test_handle_albums_error_name_id7: uses same cassette as albums_error test - test_handle_internal_error_during_download: uses cassette with 500 status during photo download Key learnings: - API errors (200 with error JSON) vs HTTP errors (4xx/5xx status) are handled differently in session.py - Download errors need HTTP status errors (500) since downloads use streaming and the error handling differs from regular JSON API calls - Removed unused PhotoLibrary and constants imports 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Download errors behave differently than API errors: - API endpoints (albums, photo listing): Can use HTTP 200 with error JSON - Download endpoints: Must use HTTP error status (4xx/5xx) because downloads use streaming and don't parse JSON responses The test behavior changes slightly: - Original mock: Raised PyiCloudAPIResponseException with "INTERNAL_ERROR" - Cassette: HTTP 500 results in "Could not find URL to download" message - Exit code changes from 1 to 0 as error is handled gracefully This is an acceptable difference as the error is still caught and handled, just reported differently. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove is_streaming_response check that was preventing exceptions for streaming downloads - Add special handling to exclude 404 errors from raising exceptions - Update cassette to return HTTP 500 for download errors - Ensure download errors raise exceptions and cause exit code 1 The is_streaming_response check was preventing the session from raising exceptions for HTTP errors on streaming downloads, causing inconsistent behavior between mock tests and cassette tests. Checking response status doesn't load the stream into memory since it's part of the HTTP headers. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Convert test_handle_internal_error_during_photo_iteration to use cassette - Convert test_handle_internal_error_during_download_name_id7 to use cassette - Add cassettes for simulating HTTP 500 errors during photo operations - Remove mock dependencies from converted tests - Clean up unused imports These tests now use actual HTTP responses via cassettes instead of mocks, providing more realistic testing of error handling behavior. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
The session error cassettes now include successful download after re-authentication, demonstrating proper retry behavior. Updated test comments to accurately reflect cassette contents and expected behavior. Both tests now: - Experience session error on first download attempt - Re-authenticate automatically - Successfully download on retry - Exit with code 0 (success) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Minor formatting improvements to session error test comments and file download expectations. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…cassettes The listing_photos_session_error_iteration.yml and listing_photos_session_error_iteration_name_id7.yml cassettes had delete file requests at the end that shouldn't be there. These have been removed. Both tests still pass after the cleanup. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…e_run - Replace PyiCloud2SARequiredException with Response2SARequired return in private libraries check - Replace PyiCloudServiceNotActivatedException with ResponseServiceNotActivated return in private libraries check - All tests pass with these minimal changes
…es check - Replace PyiCloudAPIResponseException with ResponseAPIError return - Replace PyiCloudServiceUnavailableException with ResponseServiceUnavailable return - All tests continue to pass
- Replace all four exception types with corresponding ADT returns - Maintains consistent error handling pattern - All tests pass
…h ADT return at line 1331
- Removed get_album_count function that raised exceptions - Replaced functional programming approach with direct for loop - Added inline error handling for session errors - Invalid global session errors now trigger re-authentication - Other API errors are logged and return error code directly - All tests pass, mypy strict mode passes
- Added error_occurred flag to track when retry is needed - Replaced exception raising in photo iteration with direct handling - Session errors trigger retry via flag instead of exception - WebUI errors that can be retried also use flag mechanism - Other errors return error code directly - All tests pass, mypy strict mode passes
- Replaced exception raising in download result handling with direct error handling - Used needs_retry flag to coordinate retry logic across nested loops - Session errors during download now trigger retry via flag mechanism - 500 errors and service unavailable return error code directly - Other API errors are logged but continue processing - Fixed retry logic to properly break from albums loop - All tests pass, mypy strict mode passes
- Replaced exception raising in delete photo result handling with direct error handling - Session errors during delete now trigger retry via needs_retry flag - 500 errors return error code 1 (important for delete operations) - Service unavailable returns error code 1 - Other API errors are logged but continue processing - All tests pass, mypy strict mode passes
- Replaced exception raising in autodelete result handling with direct error handling - Session errors trigger retry by continuing the while loop - WebUI errors that can be retried also continue the loop - Service unavailable returns error code 1 - Other errors return appropriate ADT results or error codes - Removed unused PyiCloud2SARequiredException import - All tests pass, mypy strict mode passes
Replace try-except block with direct ADT error handling throughout core_single_run function. All error cases now use pattern matching directly without exception catching. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…ting favorites, and overwrite metadata cli options. Combine the EXIF and XMP sidecar code into one class with tests.
…f and xmp files as funcitonality merged
Author
|
Note: This PR is based on #1301 as it requires the |
AndreyNikiforov
force-pushed
the
master
branch
from
f09784b to
dd5a421
Compare
…cal script to automate docker image creation on local dev box
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR introduces a plugin system for icloudpd that enables custom hooks at various stages of the download process, allowing users to extend functionality without modifying core code.
"As an Ai enabled power user I want to be able to write custom hooks at various stages during the download process, with the context available to icloudpd to support various functions such as adding assets to my favorite media management system."
"As an Ai enabled power user and open source believer I want my changes to be available to the world to use and to be able to maintain my code as a non invasive addition to icloudpd or as a standalone project`
Plugin Architecture
The plugin system provides a hook-based architecture with the following features:
plugins/directory) and third-party plugins installed via pip to register automaticallyImplementation Details
Core Changes:
src/icloudpd/base.py: Added plugin manager threading through the download orchestration and hook calls at appropriate lifecycle pointssrc/icloudpd/cli.py: Integrated plugin CLI argument discovery and parsingsrc/icloudpd/config.py: Added plugin configuration supportsrc/icloudpd/plugins/: New plugin infrastructure including base classes, hook protocols, and plugin managerIncluded Immich Plugin
The PR includes a fully functional Immich integration plugin (
plugins/immich/) that demonstrates real-world usage:Testing
plugins/directory in coverage reportsDocumentation
docs/plugins.md: Complete plugin development guide including architecture overview, hook reference, common patterns, testing strategies, and examplesPlugin Entry Points
Plugins register via standard Python entry points in
pyproject.toml. The demo plugin is registered as:External plugins installed via pip can register themselves similarly, allowing the plugin ecosystem to extend beyond the core repository while maintaining the option to bundle commonly-used plugins like Immich.
Backward Compatibility
All changes are backward compatible. The plugin system is opt-in via the
--pluginCLI flag and does not affect normal operation when no plugins are enabled.