From 99f527ecd1db68535a01009019738baf9da1ba9b Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Tue, 5 May 2026 21:25:27 -0700 Subject: [PATCH] =?UTF-8?q?MS=20pre-review=20polish=20followup=20=E2=80=94?= =?UTF-8?q?=20log=20levels=20+=20docstring=20honesty?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three nits I missed in the prior pass: (1) Two more exception swallows still at logger.debug — the get_recently_added_albums wrapper and the configured_clients inner loop. My earlier sed only matched single-line patterns. Both now log at warning level so a broken Plex / Jellyfin surfaces in the boot log instead of silently returning []. (2) registry.py module docstring still claimed it replaced "33 hand-maintained dispatch sites" — same overclaim I fixed on the engine docstring. Server-specific chains stay explicit in web_server.py per the "lift what's truly shared" standard; the registry just owns name → client lookup. Docstring rewritten to match reality. --- core/media_server/engine.py | 4 ++-- core/media_server/registry.py | 14 ++++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/core/media_server/engine.py b/core/media_server/engine.py index 2f22c37b..52b96b2a 100644 --- a/core/media_server/engine.py +++ b/core/media_server/engine.py @@ -233,7 +233,7 @@ class MediaServerEngine: try: return client.get_recently_added_albums(max_results) except Exception as exc: - logger.debug( + logger.warning( "%s get_recently_added_albums raised: %s", self.active_server, exc, ) @@ -256,7 +256,7 @@ class MediaServerEngine: if not hasattr(client, 'is_connected') or client.is_connected(): result[name] = client except Exception as exc: - logger.debug("%s is_connected raised in configured_clients: %s", name, exc) + logger.warning("%s is_connected raised in configured_clients: %s", name, exc) return result def reload_config(self, name: Optional[str] = None) -> bool: diff --git a/core/media_server/registry.py b/core/media_server/registry.py index 6418e002..03fe20b3 100644 --- a/core/media_server/registry.py +++ b/core/media_server/registry.py @@ -3,11 +3,17 @@ Single source of truth for which servers exist, what their canonical names are, and which client class implements each. Replaces the historic web_server.py pattern of holding 4 separate client globals -+ 33 hand-maintained ``if active_server == 'plex' / 'jellyfin' / ...`` -dispatch sites. +that every dispatch site reached individually. -Adding a new server (e.g. Subsonic, Emby) becomes one ``register`` -call here + the new client class. Web server dispatch stays put. +Server-specific dispatch chains in web_server.py (playlist add / +remove / replace, per-server metadata sync, etc.) still hand-branch +on ``active_server == X`` because the work each server does at those +sites is genuinely different — but they reach the per-server CLIENT +through ``engine.client(name)`` (which goes through this registry) +instead of separate globals. + +Adding a new server (Subsonic / Emby) = one ``register`` call here + +the new client class. """ from __future__ import annotations