Merge pull request #949 from HellRa1SeR/fix/docker-spotify-boot-hang
Fix docker spotify boot hang; add boot guard for provider clients
This commit is contained in:
commit
81bd85d639
11 changed files with 287 additions and 8 deletions
27
core/boot_phase.py
Normal file
27
core/boot_phase.py
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
"""Boot-phase guard for non-blocking container startup.
|
||||
|
||||
While the gunicorn worker is importing ``web_server`` (module-level client and
|
||||
worker initialization), external provider API probes must not block startup.
|
||||
Network validation is deferred until ``mark_boot_complete()`` runs at the end
|
||||
of that import pass.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import threading
|
||||
|
||||
_boot_lock = threading.Lock()
|
||||
_boot_active = True
|
||||
|
||||
|
||||
def is_boot_phase() -> bool:
|
||||
"""Return True while module import must avoid blocking provider API calls."""
|
||||
with _boot_lock:
|
||||
return _boot_active
|
||||
|
||||
|
||||
def mark_boot_complete() -> None:
|
||||
"""End the boot phase — provider clients may perform network probes again."""
|
||||
global _boot_active
|
||||
with _boot_lock:
|
||||
_boot_active = False
|
||||
|
|
@ -121,6 +121,7 @@ class DeezerDownloadClient(DownloadSourcePlugin):
|
|||
self._license_token = None
|
||||
self._user_data = None
|
||||
self._authenticated = False
|
||||
self._pending_arl: Optional[str] = None
|
||||
|
||||
# Quality preference
|
||||
self._quality = quality_tier_for_source('deezer', default='flac')
|
||||
|
|
@ -128,7 +129,12 @@ class DeezerDownloadClient(DownloadSourcePlugin):
|
|||
# Try to authenticate on init if ARL is configured
|
||||
arl = config_manager.get('deezer_download.arl', '')
|
||||
if arl:
|
||||
self._authenticate(arl)
|
||||
from core.boot_phase import is_boot_phase
|
||||
if is_boot_phase():
|
||||
self._pending_arl = arl
|
||||
logger.debug("Deezer ARL present — authentication deferred until after boot")
|
||||
else:
|
||||
self._authenticate(arl)
|
||||
|
||||
logger.info(f"Deezer download client initialized (download path: {self.download_path})")
|
||||
|
||||
|
|
@ -227,6 +233,11 @@ class DeezerDownloadClient(DownloadSourcePlugin):
|
|||
return self._authenticated
|
||||
|
||||
def is_authenticated(self) -> bool:
|
||||
if self._pending_arl and not self._authenticated:
|
||||
from core.boot_phase import is_boot_phase
|
||||
if not is_boot_phase():
|
||||
self._authenticate(self._pending_arl)
|
||||
self._pending_arl = None
|
||||
return self._authenticated
|
||||
|
||||
async def check_connection(self) -> bool:
|
||||
|
|
|
|||
|
|
@ -357,10 +357,26 @@ def get_hydrabase_client(allow_fallback: bool = True, require_enabled: bool = Tr
|
|||
return None
|
||||
|
||||
|
||||
def get_configured_primary_source() -> str:
|
||||
"""Return metadata.fallback_source as stored in config, without runtime downgrade.
|
||||
|
||||
Unlike get_primary_source(), this never probes Spotify auth. Use at boot and
|
||||
anywhere blocking network I/O must be avoided (gunicorn worker import, Docker
|
||||
cold start when Spotify is unreachable).
|
||||
"""
|
||||
_default = METADATA_SOURCE_PRIORITY[0]
|
||||
return _get_config_value("metadata.fallback_source", _default) or _default
|
||||
|
||||
|
||||
def get_primary_source(spotify_client_factory: Optional[MetadataClientFactory] = None) -> str:
|
||||
"""Return configured primary metadata source."""
|
||||
from core.boot_phase import is_boot_phase
|
||||
|
||||
if is_boot_phase():
|
||||
return get_configured_primary_source()
|
||||
|
||||
_default = METADATA_SOURCE_PRIORITY[0]
|
||||
source = _get_config_value("metadata.fallback_source", _default) or _default
|
||||
source = get_configured_primary_source()
|
||||
|
||||
if source == "spotify":
|
||||
try:
|
||||
|
|
@ -447,7 +463,19 @@ def get_primary_source_status(
|
|||
musicbrainz_client_factory: Optional[MetadataClientFactory] = None,
|
||||
) -> Dict[str, Any]:
|
||||
"""Return a generic status snapshot for the active primary metadata source."""
|
||||
from core.boot_phase import is_boot_phase
|
||||
|
||||
source = _get_config_value("metadata.fallback_source", "deezer") or "deezer"
|
||||
if is_boot_phase():
|
||||
display_source = source
|
||||
if source == "spotify" and _get_config_value("metadata.spotify_free", False):
|
||||
display_source = "spotify_free"
|
||||
return {
|
||||
"source": display_source,
|
||||
"connected": False,
|
||||
"response_time": 0,
|
||||
}
|
||||
|
||||
started = time.time()
|
||||
connected = False
|
||||
|
||||
|
|
@ -515,9 +543,13 @@ def get_client_for_source(
|
|||
musicbrainz_client_factory: Optional[MetadataClientFactory] = None,
|
||||
):
|
||||
"""Return exact client for a source, or None if unavailable."""
|
||||
from core.boot_phase import is_boot_phase
|
||||
|
||||
if source == "spotify":
|
||||
try:
|
||||
client = get_spotify_client(client_factory=spotify_client_factory)
|
||||
if is_boot_phase():
|
||||
return client if client and getattr(client, "sp", None) else None
|
||||
if client and client.is_spotify_authenticated():
|
||||
return client
|
||||
except Exception as e:
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ from core.metadata.registry import (
|
|||
get_discogs_client,
|
||||
get_hydrabase_client,
|
||||
get_itunes_client,
|
||||
get_configured_primary_source,
|
||||
get_primary_client,
|
||||
get_primary_source,
|
||||
get_primary_source_label,
|
||||
|
|
@ -117,6 +118,7 @@ __all__ = [
|
|||
"get_metadata_service",
|
||||
"get_musicmap_similar_artists",
|
||||
"get_primary_client",
|
||||
"get_configured_primary_source",
|
||||
"get_primary_source",
|
||||
"get_primary_source_label",
|
||||
"get_spotify_client_for_profile",
|
||||
|
|
|
|||
|
|
@ -164,6 +164,8 @@ class QobuzClient(DownloadSourcePlugin):
|
|||
|
||||
def _restore_session(self):
|
||||
"""Try to restore saved session from config."""
|
||||
from core.boot_phase import is_boot_phase
|
||||
|
||||
saved = config_manager.get('qobuz.session', {})
|
||||
app_id = saved.get('app_id', '')
|
||||
app_secret = saved.get('app_secret', '')
|
||||
|
|
@ -178,6 +180,10 @@ class QobuzClient(DownloadSourcePlugin):
|
|||
'X-User-Auth-Token': self.user_auth_token,
|
||||
})
|
||||
|
||||
if is_boot_phase():
|
||||
logger.info("Loaded Qobuz session from config (verification deferred until after boot)")
|
||||
return
|
||||
|
||||
# Verify the token is still valid
|
||||
try:
|
||||
resp = self.session.get(
|
||||
|
|
|
|||
|
|
@ -806,6 +806,16 @@ class SpotifyClient:
|
|||
self._auth_cached_result = None
|
||||
self._auth_cache_time = 0
|
||||
|
||||
def _has_cached_oauth_token(self) -> bool:
|
||||
"""Return True when a persisted OAuth token exists (no network I/O)."""
|
||||
if self.sp is None:
|
||||
return False
|
||||
try:
|
||||
cache_handler = getattr(self.sp.auth_manager, 'cache_handler', None)
|
||||
return bool(cache_handler and cache_handler.get_cached_token() is not None)
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
def is_spotify_authenticated(self) -> bool:
|
||||
"""Check if Spotify client is specifically authenticated (not just iTunes fallback).
|
||||
Results are cached for 60 seconds to avoid excessive API calls.
|
||||
|
|
@ -881,6 +891,26 @@ class SpotifyClient:
|
|||
logger.debug("publish_spotify_status cache hit: %s", e)
|
||||
return self._auth_cached_result
|
||||
|
||||
from core.boot_phase import is_boot_phase
|
||||
if is_boot_phase():
|
||||
result = self._has_cached_oauth_token()
|
||||
with self._auth_cache_lock:
|
||||
self._auth_cached_result = result
|
||||
self._auth_cache_time = time.time()
|
||||
try:
|
||||
from core.metadata.status import publish_spotify_status
|
||||
|
||||
publish_spotify_status(
|
||||
connected=result,
|
||||
authenticated=result,
|
||||
rate_limited=False,
|
||||
rate_limit=None,
|
||||
post_ban_cooldown=None,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.debug("publish_spotify_status boot-phase: %s", e)
|
||||
return result
|
||||
|
||||
# Cache miss — make API call outside the lock.
|
||||
# Safety: if there's no cached token, return False immediately.
|
||||
# Without this guard, spotipy's auth_manager will try to start an interactive
|
||||
|
|
@ -912,7 +942,8 @@ class SpotifyClient:
|
|||
# Use a dedicated probe client (retries=0) so a 429 here propagates
|
||||
# immediately and we can detect long Retry-After bans.
|
||||
try:
|
||||
probe = spotipy.Spotify(auth_manager=self.sp.auth_manager, retries=0)
|
||||
probe = spotipy.Spotify(
|
||||
auth_manager=self.sp.auth_manager, retries=0, requests_timeout=15)
|
||||
probe.current_user()
|
||||
result = True
|
||||
except Exception as e:
|
||||
|
|
|
|||
|
|
@ -520,10 +520,14 @@ class TidalClient:
|
|||
|
||||
return True
|
||||
|
||||
def is_authenticated(self):
|
||||
def is_authenticated(self) -> bool:
|
||||
"""Check if client is authenticated, refreshing expired tokens if possible"""
|
||||
if self.access_token and time.time() < self.token_expires_at:
|
||||
return True
|
||||
from core.boot_phase import is_boot_phase
|
||||
|
||||
if is_boot_phase():
|
||||
if self.access_token and time.time() < self.token_expires_at:
|
||||
return True
|
||||
return bool(self.access_token and self.refresh_token)
|
||||
|
||||
# Backoff: if refresh recently failed, don't retry for 5 minutes
|
||||
if hasattr(self, '_refresh_failed_at') and self._refresh_failed_at:
|
||||
|
|
|
|||
|
|
@ -134,6 +134,7 @@ class TidalDownloadClient(DownloadSourcePlugin):
|
|||
|
||||
self._device_auth_future = None
|
||||
self._device_auth_link = None
|
||||
self._boot_session_tokens: Optional[dict] = None
|
||||
|
||||
# Engine reference is populated by set_engine() at registration
|
||||
# time. Until then dispatch returns None — orchestrator wires
|
||||
|
|
@ -163,6 +164,14 @@ class TidalDownloadClient(DownloadSourcePlugin):
|
|||
expiry_time = saved.get('expiry_time', 0)
|
||||
|
||||
if token_type and access_token:
|
||||
from core.boot_phase import is_boot_phase
|
||||
if is_boot_phase():
|
||||
self._boot_session_tokens = saved
|
||||
logger.info(
|
||||
"Loaded Tidal download session from config (verification deferred until after boot)"
|
||||
)
|
||||
return
|
||||
|
||||
try:
|
||||
expiry_dt = datetime.fromtimestamp(expiry_time, tz=timezone.utc) if expiry_time else None
|
||||
|
||||
|
|
@ -181,6 +190,39 @@ class TidalDownloadClient(DownloadSourcePlugin):
|
|||
except Exception as e:
|
||||
logger.warning(f"Could not restore Tidal session: {e}")
|
||||
|
||||
def _complete_deferred_session(self) -> bool:
|
||||
"""Finish restoring a session that was deferred during boot."""
|
||||
pending = getattr(self, '_boot_session_tokens', None)
|
||||
if not pending or tidalapi is None:
|
||||
self._boot_session_tokens = None
|
||||
return False
|
||||
|
||||
if not self.session:
|
||||
self.session = tidalapi.Session()
|
||||
|
||||
token_type = pending.get('token_type', '')
|
||||
access_token = pending.get('access_token', '')
|
||||
refresh_token = pending.get('refresh_token', '')
|
||||
expiry_time = pending.get('expiry_time', 0)
|
||||
self._boot_session_tokens = None
|
||||
|
||||
try:
|
||||
expiry_dt = datetime.fromtimestamp(expiry_time, tz=timezone.utc) if expiry_time else None
|
||||
restored = self.session.load_oauth_session(
|
||||
token_type=token_type,
|
||||
access_token=access_token,
|
||||
refresh_token=refresh_token,
|
||||
expiry_time=expiry_dt,
|
||||
)
|
||||
if restored and self.session.check_login():
|
||||
logger.info("Restored Tidal download session from saved tokens")
|
||||
self._save_session()
|
||||
return True
|
||||
logger.warning("Saved Tidal session tokens are invalid/expired")
|
||||
except Exception as e:
|
||||
logger.warning(f"Could not restore Tidal session: {e}")
|
||||
return False
|
||||
|
||||
def _save_session(self):
|
||||
if not self.session:
|
||||
return
|
||||
|
|
@ -192,6 +234,14 @@ class TidalDownloadClient(DownloadSourcePlugin):
|
|||
})
|
||||
|
||||
def is_authenticated(self) -> bool:
|
||||
from core.boot_phase import is_boot_phase
|
||||
if is_boot_phase():
|
||||
pending = getattr(self, '_boot_session_tokens', None)
|
||||
return bool(pending and pending.get('access_token'))
|
||||
|
||||
if getattr(self, '_boot_session_tokens', None):
|
||||
return self._complete_deferred_session()
|
||||
|
||||
if not self.session:
|
||||
return False
|
||||
try:
|
||||
|
|
|
|||
77
tests/metadata/test_boot_phase_providers.py
Normal file
77
tests/metadata/test_boot_phase_providers.py
Normal file
|
|
@ -0,0 +1,77 @@
|
|||
"""Boot-phase guards must defer blocking provider network probes."""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from core.boot_phase import is_boot_phase, mark_boot_complete
|
||||
from core.metadata import registry
|
||||
|
||||
|
||||
def setup_function():
|
||||
mark_boot_complete()
|
||||
|
||||
|
||||
def teardown_function():
|
||||
mark_boot_complete()
|
||||
|
||||
|
||||
def test_get_primary_source_skips_spotify_probe_during_boot(monkeypatch):
|
||||
import core.boot_phase as boot_phase
|
||||
|
||||
boot_phase._boot_active = True
|
||||
monkeypatch.setattr(registry, "get_configured_primary_source", lambda: "spotify")
|
||||
|
||||
with patch.object(registry, "get_spotify_client") as get_client:
|
||||
assert registry.get_primary_source() == "spotify"
|
||||
get_client.assert_not_called()
|
||||
|
||||
|
||||
def test_get_primary_source_status_skips_client_probe_during_boot(monkeypatch):
|
||||
import core.boot_phase as boot_phase
|
||||
|
||||
boot_phase._boot_active = True
|
||||
monkeypatch.setattr(
|
||||
registry, "_get_config_value",
|
||||
lambda key, default=None: "spotify" if key == "metadata.fallback_source" else default,
|
||||
)
|
||||
|
||||
with patch.object(registry, "get_client_for_source") as get_client:
|
||||
status = registry.get_primary_source_status()
|
||||
get_client.assert_not_called()
|
||||
assert status["source"] == "spotify"
|
||||
assert status["connected"] is False
|
||||
|
||||
|
||||
def test_spotify_auth_uses_token_presence_only_during_boot(monkeypatch):
|
||||
import core.boot_phase as boot_phase
|
||||
from core.spotify_client import SpotifyClient
|
||||
|
||||
boot_phase._boot_active = True
|
||||
client = SpotifyClient.__new__(SpotifyClient)
|
||||
client.sp = MagicMock()
|
||||
client._auth_cache_lock = __import__('threading').Lock()
|
||||
client._auth_cached_result = None
|
||||
client._auth_cache_time = 0
|
||||
client._AUTH_CACHE_TTL = 900
|
||||
|
||||
monkeypatch.setattr(client, "_has_cached_oauth_token", lambda: True)
|
||||
|
||||
with patch("spotipy.Spotify") as spotify_cls:
|
||||
assert client.is_spotify_authenticated() is True
|
||||
spotify_cls.assert_not_called()
|
||||
|
||||
|
||||
def test_deezer_download_defers_arl_auth_during_boot(monkeypatch):
|
||||
import core.boot_phase as boot_phase
|
||||
from core.deezer_download_client import DeezerDownloadClient
|
||||
|
||||
boot_phase._boot_active = True
|
||||
monkeypatch.setattr(
|
||||
"config.settings.config_manager.get",
|
||||
lambda key, default=None: "fake-arl" if key == "deezer_download.arl" else default,
|
||||
)
|
||||
|
||||
with patch.object(DeezerDownloadClient, "_authenticate") as authenticate:
|
||||
client = DeezerDownloadClient(download_path="/tmp/deezer-test")
|
||||
authenticate.assert_not_called()
|
||||
assert client._pending_arl == "fake-arl"
|
||||
assert client.is_authenticated() is False
|
||||
32
tests/metadata/test_configured_primary_source.py
Normal file
32
tests/metadata/test_configured_primary_source.py
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
"""Tests for boot-safe configured primary source lookup."""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from core.boot_phase import mark_boot_complete
|
||||
from core.metadata import registry
|
||||
|
||||
|
||||
def setup_function():
|
||||
mark_boot_complete()
|
||||
|
||||
|
||||
def test_get_configured_primary_source_reads_config_without_auth_probe(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
registry,
|
||||
"_get_config_value",
|
||||
lambda key, default=None: "spotify" if key == "metadata.fallback_source" else default,
|
||||
)
|
||||
|
||||
with patch.object(registry, "get_spotify_client") as get_client:
|
||||
assert registry.get_configured_primary_source() == "spotify"
|
||||
get_client.assert_not_called()
|
||||
|
||||
|
||||
def test_get_primary_source_still_downgrades_unauthenticated_spotify(monkeypatch):
|
||||
monkeypatch.setattr(registry, "get_configured_primary_source", lambda: "spotify")
|
||||
|
||||
spotify = MagicMock()
|
||||
spotify.is_spotify_authenticated.return_value = False
|
||||
monkeypatch.setattr(registry, "get_spotify_client", lambda **_: spotify)
|
||||
|
||||
assert registry.get_primary_source() == registry.METADATA_SOURCE_PRIORITY[0]
|
||||
|
|
@ -36535,11 +36535,13 @@ except Exception as e:
|
|||
# they explicitly want background Spotify enrichment.
|
||||
spotify_enrichment_worker = None
|
||||
try:
|
||||
from core.metadata_service import get_primary_source as _get_primary_source
|
||||
from core.metadata_service import get_configured_primary_source
|
||||
from database.music_database import MusicDatabase
|
||||
spotify_enrichment_db = MusicDatabase()
|
||||
spotify_enrichment_worker = SpotifyWorker(database=spotify_enrichment_db)
|
||||
_primary = _get_primary_source()
|
||||
# Use configured source only — get_primary_source() probes Spotify auth and can
|
||||
# block gunicorn worker boot indefinitely when the API is unreachable.
|
||||
_primary = get_configured_primary_source()
|
||||
_user_paused = config_manager.get('spotify_enrichment_paused', False)
|
||||
if _user_paused or _primary != 'spotify':
|
||||
spotify_enrichment_worker.paused = True # Set BEFORE start() to prevent race condition
|
||||
|
|
@ -39025,6 +39027,11 @@ def start_runtime_services():
|
|||
_runtime_started = True
|
||||
|
||||
|
||||
# Module import is complete — provider clients may now perform network probes.
|
||||
from core.boot_phase import mark_boot_complete
|
||||
mark_boot_complete()
|
||||
|
||||
|
||||
# Direct execution: python web_server.py (dev/Windows fallback)
|
||||
# Production should use: gunicorn -c gunicorn.conf.py wsgi:application
|
||||
if _DIRECT_RUN:
|
||||
|
|
|
|||
Loading…
Reference in a new issue