Repository navigation
Conversation
Continue Watching + Next Up (plus next N episodes) of every Jellyfin user are cached alongside Plex OnDeck; files playing on Jellyfin are skipped. Jellyfin paths are translated into Plex paths via jellyfin_path_mappings. Move-back is skipped when Jellyfin data is incomplete. Configured via settings keys or JELLYFIN_* env vars. Refs: https://nextcloud.sikorski.cloud/index.php/apps/deck/board/78/card/258 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
feat: Jellyfin as additional OnDeck source
|
@Brandon-Haney |
Brandon-Haney
left a comment
Brandon-Haney
left a comment
There was a problem hiding this comment.
Thanks for putting this together. Mapping Jellyfin paths into Plex paths is a good fit, and with Jellyfin unconfigured it stays out of the way of existing users. I've read through the code but haven't run it, and CI hasn't run yet either.
There are some things I'd want fixed before we seriously consider merging this. Most of the detail is in the inline comments:
- Sessions: name the server the session was found on, and count sessions on unmapped paths.
- A failed Jellyfin request shouldn't drop a user's whole list.
- Long shows are at risk of hitting the 30s timeout.
- Validate and normalize the path mappings.
- Log unmapped paths at INFO.
- State the minimum Jellyfin version in the README.
One more that can't go inline because the file isn't in this diff: export_settings(include_sensitive=False) in web/services/settings_service.py redacts from an explicit list, so jellyfin_api_key and jellyfin_url would stay in exports people share. Please blank both there, the same way PLEX_TOKEN and PLEX_URL are handled, and add a test for it.
Also, jellyfin_skip_users is the only user filter here. With users_toggle off, Plex only looks at the main account, but Jellyfin still fetches everyone. I think those should match, or the README should say that they don't.
My open PR #228 adds a test requiring every settings key to be in KNOWN_SETTINGS_KEYS, and REDACTED_EXPORT_KEYS needs the secret ones too. Those don't exist on main yet, so whichever PR merges second will pick that up.
I'm not asking for a Settings UI right now. That's a separate conversation if this moves forward. Please add tests alongside the fixes, especially for items 1 and 2.
| """Check for active Plex sessions.""" | ||
| """Check for active Plex and Jellyfin sessions.""" | ||
| sessions = self.plex_manager.get_active_sessions() | ||
| jellyfin_paths = self.jellyfin_manager.get_active_session_paths() if self.jellyfin_manager else [] |
There was a problem hiding this comment.
A few things here:
The messages should name the server that actually has the session. Right now the generic message means Plex and the other one says Jellyfin. Something like this would cover Plex, Jellyfin or both:
active_servers = [name for name, found in (("Plex", sessions), ("Jellyfin", jellyfin_sessions)) if found]
if active_servers:
message = f"There is an active session on {' and '.join(active_servers)}. Exiting..."get_active_session_paths() only returns mapped paths, so a session on an unmapped path looks like no session and exit_if_active_session doesn't fire. It would help to have it report the session separately from the paths.
The [FILTER] Skipped N active session(s) line sits inside if sessions, so Jellyfin-only skips never show up at INFO. It should cover both.
The log analyzer also matches this text. docs/index.html has /There is an active session\. Exiting/i, which should become /There is an active session.*Exiting/i. The timeline pattern /active session.*Exiting/i already works with the wording above, but neither one matches "active Jellyfin session" as it's written now.
| logging.debug(f"[JELLYFIN] [USER:{item.username}] OnDeck found: {item.file_path}") | ||
| return on_deck_files | ||
|
|
||
| def _fetch_user_on_deck_media(self, user_id: str, username: str, days_to_monitor: int, |
There was a problem hiding this comment.
The list only comes back at the end, so one failed request loses everything already collected for that user. Could each item get its own try/except, so the failure only costs that item and still marks the data incomplete? We made the same kind of change on the Plex side in #204.
It's also worth remembering that incomplete Jellyfin data skips array restore for Plex files as well. I think that's the right call for safety, but the warning should make it obvious that Jellyfin is what's holding things up, especially for people running it as a backup server.
| logging.warning(f"[JELLYFIN] Skipping next episode fetch for '{show}' - missing index data") | ||
| return | ||
|
|
||
| episodes = self._get(f"/Shows/{item['SeriesId']}/Episodes", userId=user_id, |
There was a problem hiding this comment.
This pulls every episode of the show with MediaSources, once per Next Up item and per user, under a 30s timeout. A long-running show on a slow NAS could plausibly time out, and that then runs into the problem above. /Shows/{seriesId}/Episodes supports startItemId, so passing the current episode would skip everything before it.
| skip_users: Optional[List[str]] = None, session: Optional[Any] = None): | ||
| self.url = url.rstrip('/') | ||
| # Longest prefix first, so nested mappings win | ||
| self.path_mappings = sorted((path_mappings or {}).items(), key=lambda kv: len(kv[0]), reverse=True) |
There was a problem hiding this comment.
The prefixes aren't normalized. {"/media": "/mnt/user/plex-media/"} also matches /media2/... and produces // in the path, and I'd expect that to carry through into the tracker and exclude list entries. Could this normalize trailing slashes on both sides?
| env_mappings = os.environ.get('JELLYFIN_PATH_MAPPINGS') | ||
| if env_mappings: | ||
| try: | ||
| self.jellyfin.path_mappings = json.loads(env_mappings) |
There was a problem hiding this comment.
Valid JSON that isn't an object, like [], gets past this and then raises on .items() when the manager is created. A bad setting on an optional integration shouldn't stop a Plex run, so please check it's a dict and log an error if it isn't.
| if plex_path: | ||
| result.append(plex_path) | ||
| else: | ||
| logging.debug(f"[JELLYFIN] No jellyfin_path_mappings entry for {path}, skipping") |
There was a problem hiding this comment.
This should be INFO, as Plex's [CONFIG] Skipping unmapped path is. At DEBUG, a wrong mapping just looks like "Jellyfin OnDeck: 0 items" and nothing tells the user why. One line per library prefix would be enough.
|
|
||
| ## Jellyfin (optional) | ||
|
|
||
| Jellyfin can be added as a second OnDeck source next to Plex, for setups where both servers share the same media. Every enabled Jellyfin user's *Continue Watching* and *Next Up* (plus the next `number_episodes` episodes) are cached like Plex OnDeck items, and files playing on Jellyfin are skipped. There is no Jellyfin watchlist. Plex stays required. |
There was a problem hiding this comment.
Please add that this needs Jellyfin 10.9 or newer. /UserItems/Resume isn't in 10.8.13, so on an older server every user fetch would 404, the data would count as incomplete, and nothing would ever move back to the array.
- Name the server(s) with an active session in the exit message and count Jellyfin sessions on unmapped paths; log [FILTER] skips for Jellyfin too - Per-item and per-list error handling, so one failed request only costs that item while still marking the data incomplete - Fetch episodes from the current one on (startItemId) to keep long shows under the request timeout - Normalize trailing slashes in jellyfin_path_mappings, match whole folders only, and reject non-object/invalid mappings without stopping the run - Log unmapped paths at INFO once per library folder - Redact jellyfin_api_key and jellyfin_url in sensitive-free exports - README: Jellyfin 10.9+, users_toggle scope, incomplete-data behavior - Log analyzer matches the new active-session message Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the quick turnaround. I went back through the new commit and everything from the first review is covered, with tests for each, so I'm happy with where this is. Two small things, neither blocking:
Since neither Nirin nor I run Jellyfin, could you paste a I'll sort out the |
PR StudioNirin#234 adds four jellyfin_* settings keys. Listing them here keeps test_every_real_settings_key_is_known passing whichever of the two PRs merges first, and keeps an imported redacted export from replacing the installation's Jellyfin URL and API key.
PR StudioNirin#234 adds four jellyfin_* settings keys. Listing them here keeps test_every_real_settings_key_is_known passing whichever of the two PRs merges first, and keeps an imported redacted export from replacing the installation's Jellyfin URL and API key.
…in#234) - Accept jellyfin_skip_users as a comma-separated string in the settings file, so "guest,kid" no longer turns into a set of single characters - Retry the episode list without startItemId when it comes back empty; Jellyfin <= 10.11 returns nothing when the current item is an alternate version Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks! Both points are in 02ba6c7:
Logs below are from real Run 1: nothing playing on Jellyfin (a Plex session was active) Run 2: Family Guy playing on Jellyfin, Plex session still active Neither run logged any unmapped paths, |
Jellyfin has no watchlist. With jellyfin_favorites_toggle (and watchlist_toggle) on, unplayed favorite movies and the first watchlist_episodes unplayed episodes of favorite series (without specials) go through the regular watchlist path: tracker, retention, move-back after watching. Retention counts from the first run that sees a favorite, since Jellyfin has no favorited-at date. A failed favorites request skips the array restore, like incomplete OnDeck data. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Jellyfin don't have a watchlist. But have a favorites list. With the newest commit the favorites will be used like Plex is used with watchlist. Which is useful for movie caching. For me its working fine on my Jellyfin instance. |
PexCache-D extended with Jellyfin. It works here fine. Here we have at home Plex as main player and Jellyfin as backup and because I'm curious to see how Jellyfin progress.
The entire code is written by Claude because I'm a sysadmin and not a programmer. I'm completely fine with it if you disregard this pull request because of AI or code quality or etc. At least it will be findable over the search for other Jellyfin user.