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.
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.