Fix #523: paginate Jellyfin get_all_songs; raise instead of truncating#591
Conversation
… truncating Jellyfin 10.11.x times out on a single unbounded /Items query for large libraries (issue NeptuneHub#523) -- the reporter saw it fail even at REQUESTS_TIMEOUT=1200. Paginate into 500-item pages (mirroring Emby) so each request stays bounded. On a page failure, both Jellyfin and Emby now re-raise instead of returning a partial (Emby break) or empty (Jellyfin return []) list. get_all_songs feeds the migration matcher, where any score row missing from the returned set is deleted as an orphan by the execute step; a silently truncated scan would destroy real analysis data. The sole caller (provider_probe.fetch_all_tracks via the migration routes) already wraps the call and surfaces the error as an HTTP 500 to the user. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces pagination to Jellyfin's get_all_songs to prevent HTTP timeouts on large libraries, and updates both Jellyfin and Emby implementations to raise exceptions on failures instead of returning partial lists (which could lead to data loss during migrations). Comprehensive unit tests are added to verify pagination and error propagation. A review comment suggests using r.json().get("Items") or [] in Jellyfin's implementation to prevent a potential TypeError if the API returns a null value for the Items key.
|
I don't remember, can you confirm that this is ready to be merged? |
|
Yes sir! |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces pagination to Jellyfin's get_all_songs function to prevent HTTP read timeouts on large libraries. Additionally, both Jellyfin and Emby implementations of get_all_songs are updated to raise exceptions on failure rather than returning a partial list, preventing silent data truncation and potential loss of analysis data. Comprehensive unit tests have been added to verify pagination and error-handling behaviors. No review comments were provided, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
AS-IS
Jellyfin 10.11.x times out on a single unbounded /Items query for large libraries (issue #523), the reporter saw it fail even at REQUESTS_TIMEOUT=1200. Paginate into 500-item pages (mirroring Emby) so each request stays bounded.
TO-BE
On a page failure, both Jellyfin and Emby now re-raise instead of returning a partial (Emby break) or empty (Jellyfin return []) list. get_all_songs feeds the migration matcher, where any score row missing from the returned set is deleted as an orphan by the execute step; a silently truncated scan would destroy real analysis data. The sole caller (provider_probe.fetch_all_tracks via the migration routes) already wraps the call and surfaces the error as an HTTP 500 to the user.
Test
Tested on Jellyfin and Emby
Other useful information
Checklist
Type of change:
Tested on architecture:
Tested on media server:
Updated:
Other:
Related ISSUE: Closes #523