Refactor similar artist backfill
Switch similar-artist backfill to the shared provider-priority flow instead of assuming iTunes as the fallback. Reuse the generic metadata search helpers, keep a compatibility alias for the old helper name, and update the scanner tests to cover the new path. Add a regression test that verifies backfill walks each available fallback provider and persists the resolved IDs per source.
This commit is contained in:
parent
47a6c257ad
commit
8382b8e247
2 changed files with 115 additions and 102 deletions
|
|
@ -30,63 +30,6 @@ DELAY_BETWEEN_ARTISTS = 4.0 # 4 seconds between different artists (was 2s,
|
||||||
DELAY_BETWEEN_ALBUMS = 0.5 # 500ms between albums for same artist
|
DELAY_BETWEEN_ALBUMS = 0.5 # 500ms between albums for same artist
|
||||||
DELAY_BETWEEN_API_BATCHES = 1.0 # 1 second between API batch operations
|
DELAY_BETWEEN_API_BATCHES = 1.0 # 1 second between API batch operations
|
||||||
|
|
||||||
# iTunes API retry configuration
|
|
||||||
ITUNES_MAX_RETRIES = 3
|
|
||||||
ITUNES_BASE_DELAY = 1.0 # Base delay in seconds for exponential backoff
|
|
||||||
|
|
||||||
|
|
||||||
def _get_fallback_metadata_client():
|
|
||||||
"""Get the configured metadata client — delegates to centralized metadata_service."""
|
|
||||||
from core.metadata_service import get_primary_source, get_primary_client
|
|
||||||
return get_primary_client(), get_primary_source()
|
|
||||||
|
|
||||||
|
|
||||||
def itunes_api_call_with_retry(func, *args, max_retries=ITUNES_MAX_RETRIES, **kwargs):
|
|
||||||
"""
|
|
||||||
Execute an iTunes API call with exponential backoff retry logic.
|
|
||||||
|
|
||||||
Args:
|
|
||||||
func: The function to call
|
|
||||||
*args: Arguments to pass to the function
|
|
||||||
max_retries: Maximum number of retry attempts
|
|
||||||
**kwargs: Keyword arguments to pass to the function
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
The result of the function call, or None if all retries failed
|
|
||||||
"""
|
|
||||||
last_error = None
|
|
||||||
for attempt in range(max_retries):
|
|
||||||
try:
|
|
||||||
result = func(*args, **kwargs)
|
|
||||||
return result
|
|
||||||
except requests.exceptions.HTTPError as e:
|
|
||||||
# Handle rate limiting (429) and server errors (5xx)
|
|
||||||
if e.response is not None and e.response.status_code == 429:
|
|
||||||
delay = ITUNES_BASE_DELAY * (2 ** attempt)
|
|
||||||
logger.warning(f"[iTunes] Rate limited, retrying in {delay}s (attempt {attempt + 1}/{max_retries})")
|
|
||||||
time.sleep(delay)
|
|
||||||
last_error = e
|
|
||||||
elif e.response is not None and e.response.status_code >= 500:
|
|
||||||
delay = ITUNES_BASE_DELAY * (2 ** attempt)
|
|
||||||
logger.warning(f"[iTunes] Server error {e.response.status_code}, retrying in {delay}s (attempt {attempt + 1}/{max_retries})")
|
|
||||||
time.sleep(delay)
|
|
||||||
last_error = e
|
|
||||||
else:
|
|
||||||
raise # Don't retry on client errors (4xx except 429)
|
|
||||||
except requests.exceptions.RequestException as e:
|
|
||||||
# Retry on connection errors
|
|
||||||
delay = ITUNES_BASE_DELAY * (2 ** attempt)
|
|
||||||
logger.warning(f"[iTunes] Connection error, retrying in {delay}s (attempt {attempt + 1}/{max_retries}): {e}")
|
|
||||||
time.sleep(delay)
|
|
||||||
last_error = e
|
|
||||||
except Exception as e:
|
|
||||||
# Don't retry on other exceptions
|
|
||||||
raise
|
|
||||||
|
|
||||||
if last_error:
|
|
||||||
logger.error(f"[iTunes] All {max_retries} retry attempts failed: {last_error}")
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def clean_track_name_for_search(track_name):
|
def clean_track_name_for_search(track_name):
|
||||||
"""
|
"""
|
||||||
|
|
@ -1315,7 +1258,7 @@ class WatchlistScanner:
|
||||||
spotify_authenticated = self.spotify_client and self.spotify_client.is_spotify_authenticated()
|
spotify_authenticated = self.spotify_client and self.spotify_client.is_spotify_authenticated()
|
||||||
if self.database.has_fresh_similar_artists(source_artist_id, days_threshold=30, require_spotify=spotify_authenticated, profile_id=artist_profile_id):
|
if self.database.has_fresh_similar_artists(source_artist_id, days_threshold=30, require_spotify=spotify_authenticated, profile_id=artist_profile_id):
|
||||||
logger.info("Similar artists for %s are cached and fresh (profile %s)", artist.artist_name, artist_profile_id)
|
logger.info("Similar artists for %s are cached and fresh (profile %s)", artist.artist_name, artist_profile_id)
|
||||||
self._backfill_similar_artists_itunes_ids(source_artist_id, profile_id=artist_profile_id)
|
self._backfill_similar_artists_fallback_ids(source_artist_id, profile_id=artist_profile_id)
|
||||||
else:
|
else:
|
||||||
logger.info("Fetching similar artists for %s (profile %s)...", artist.artist_name, artist_profile_id)
|
logger.info("Fetching similar artists for %s (profile %s)...", artist.artist_name, artist_profile_id)
|
||||||
self.update_similar_artists(artist, profile_id=artist_profile_id, source_artist_id=source_artist_id)
|
self.update_similar_artists(artist, profile_id=artist_profile_id, source_artist_id=source_artist_id)
|
||||||
|
|
@ -2313,57 +2256,79 @@ class WatchlistScanner:
|
||||||
logger.error(f"Error fetching similar artists from MusicMap: {e}")
|
logger.error(f"Error fetching similar artists from MusicMap: {e}")
|
||||||
return []
|
return []
|
||||||
|
|
||||||
def _backfill_similar_artists_itunes_ids(self, source_artist_id: str, profile_id: int = 1) -> int:
|
def _update_similar_artist_source_id(self, similar_artist_id: int, source: str, source_id: str) -> bool:
|
||||||
"""
|
"""Persist a resolved similar-artist ID for a supported source."""
|
||||||
Backfill missing iTunes IDs for cached similar artists.
|
if source == 'deezer':
|
||||||
This ensures seamless dual-source support without clearing cached data.
|
return self.database.update_similar_artist_deezer_id(similar_artist_id, source_id)
|
||||||
|
if source == 'itunes':
|
||||||
|
return self.database.update_similar_artist_itunes_id(similar_artist_id, source_id)
|
||||||
|
return False
|
||||||
|
|
||||||
Args:
|
def _backfill_similar_artists_fallback_ids(self, source_artist_id: str, profile_id: int = 1) -> int:
|
||||||
source_artist_id: The source artist ID to backfill similar artists for
|
|
||||||
profile_id: Profile to scope the backfill to
|
|
||||||
|
|
||||||
Returns:
|
|
||||||
Number of similar artists updated with iTunes IDs
|
|
||||||
"""
|
"""
|
||||||
|
Backfill missing fallback-provider IDs for cached similar artists.
|
||||||
|
|
||||||
|
Uses the configured source priority, filtered to providers that have
|
||||||
|
writable similar-artist ID columns. This keeps old cached rows usable
|
||||||
|
when the active metadata provider changes.
|
||||||
|
"""
|
||||||
|
backfill_sources = [source for source in self._discovery_source_priority() if source in {'itunes', 'deezer'}]
|
||||||
|
if not backfill_sources:
|
||||||
|
logger.debug("No fallback metadata providers available for similar-artist backfill")
|
||||||
|
return 0
|
||||||
|
|
||||||
|
updated_total = 0
|
||||||
|
|
||||||
try:
|
try:
|
||||||
# Get fallback metadata client (iTunes or Deezer)
|
for source in backfill_sources:
|
||||||
fallback_client, fallback_source = _get_fallback_metadata_client()
|
client = get_client_for_source(source)
|
||||||
|
if not client:
|
||||||
# Get similar artists that are missing IDs for the active fallback source
|
logger.debug("Skipping %s similar-artist backfill - client unavailable", source)
|
||||||
similar_artists = self.database.get_similar_artists_missing_fallback_ids(source_artist_id, fallback_source, profile_id=profile_id)
|
|
||||||
|
|
||||||
if not similar_artists:
|
|
||||||
return 0
|
|
||||||
|
|
||||||
logger.info(f"Backfilling {fallback_source} IDs for {len(similar_artists)} similar artists")
|
|
||||||
|
|
||||||
updated_count = 0
|
|
||||||
for similar_artist in similar_artists:
|
|
||||||
try:
|
|
||||||
results = fallback_client.search_artists(similar_artist.similar_artist_name, limit=1)
|
|
||||||
if results and len(results) > 0:
|
|
||||||
found_id = results[0].id
|
|
||||||
# Update the similar artist with the correct source ID
|
|
||||||
if fallback_source == 'deezer':
|
|
||||||
success = self.database.update_similar_artist_deezer_id(similar_artist.id, found_id)
|
|
||||||
else:
|
|
||||||
success = self.database.update_similar_artist_itunes_id(similar_artist.id, found_id)
|
|
||||||
if success:
|
|
||||||
updated_count += 1
|
|
||||||
logger.debug(f" Backfilled {fallback_source} ID {found_id} for {similar_artist.similar_artist_name}")
|
|
||||||
except Exception as e:
|
|
||||||
logger.debug(f" Could not backfill {fallback_source} ID for {similar_artist.similar_artist_name}: {e}")
|
|
||||||
continue
|
continue
|
||||||
|
|
||||||
if updated_count > 0:
|
similar_artists = self.database.get_similar_artists_missing_fallback_ids(
|
||||||
logger.info(f"Backfilled {updated_count} similar artists with {fallback_source} IDs")
|
source_artist_id,
|
||||||
|
source,
|
||||||
|
profile_id=profile_id,
|
||||||
|
)
|
||||||
|
if not similar_artists:
|
||||||
|
continue
|
||||||
|
|
||||||
return updated_count
|
logger.info("Backfilling %s IDs for %s similar artists", source, len(similar_artists))
|
||||||
|
|
||||||
|
updated_count = 0
|
||||||
|
for similar_artist in similar_artists:
|
||||||
|
try:
|
||||||
|
results = self._search_artists_for_source(source, similar_artist.similar_artist_name, limit=1, client=client)
|
||||||
|
if not results:
|
||||||
|
continue
|
||||||
|
|
||||||
|
found_id = self._extract_entity_id(results[0])
|
||||||
|
if not found_id:
|
||||||
|
continue
|
||||||
|
|
||||||
|
success = self._update_similar_artist_source_id(similar_artist.id, source, found_id)
|
||||||
|
if success:
|
||||||
|
updated_count += 1
|
||||||
|
updated_total += 1
|
||||||
|
logger.debug(" Backfilled %s ID %s for %s", source, found_id, similar_artist.similar_artist_name)
|
||||||
|
except Exception as e:
|
||||||
|
logger.debug(" Could not backfill %s ID for %s: %s", source, similar_artist.similar_artist_name, e)
|
||||||
|
continue
|
||||||
|
|
||||||
|
if updated_count > 0:
|
||||||
|
logger.info("Backfilled %s similar artists with %s IDs", updated_count, source)
|
||||||
|
|
||||||
|
return updated_total
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.error(f"Error backfilling similar artists {fallback_source if 'fallback_source' in dir() else 'fallback'} IDs: {e}")
|
logger.error("Error backfilling similar artists IDs: %s", e)
|
||||||
return 0
|
return 0
|
||||||
|
|
||||||
|
def _backfill_similar_artists_itunes_ids(self, source_artist_id: str, profile_id: int = 1) -> int:
|
||||||
|
"""Backward-compatible alias for the provider-priority backfill path."""
|
||||||
|
return self._backfill_similar_artists_fallback_ids(source_artist_id, profile_id=profile_id)
|
||||||
|
|
||||||
def update_similar_artists(
|
def update_similar_artists(
|
||||||
self,
|
self,
|
||||||
watchlist_artist: WatchlistArtist,
|
watchlist_artist: WatchlistArtist,
|
||||||
|
|
|
||||||
|
|
@ -296,6 +296,54 @@ def test_fetch_similar_artists_from_musicmap_uses_provider_priority(monkeypatch)
|
||||||
assert spotify_client.search_calls[-1][2]["allow_fallback"] is False
|
assert spotify_client.search_calls[-1][2]["allow_fallback"] is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_backfill_similar_artists_fallback_ids_uses_provider_priority(monkeypatch):
|
||||||
|
def make_client(source):
|
||||||
|
client = types.SimpleNamespace(search_calls=[])
|
||||||
|
|
||||||
|
def search_artists(query, limit=1, **kwargs):
|
||||||
|
client.search_calls.append((query, limit, kwargs))
|
||||||
|
safe_name = query.lower().replace(" ", "-")
|
||||||
|
return [types.SimpleNamespace(id=f"{source}-{safe_name}", name=query)]
|
||||||
|
|
||||||
|
client.search_artists = search_artists
|
||||||
|
return client
|
||||||
|
|
||||||
|
deezer_client = make_client("deezer")
|
||||||
|
itunes_client = make_client("itunes")
|
||||||
|
|
||||||
|
deezer_artist = types.SimpleNamespace(id=11, similar_artist_name="Deezer Artist")
|
||||||
|
itunes_artist = types.SimpleNamespace(id=22, similar_artist_name="iTunes Artist")
|
||||||
|
|
||||||
|
monkeypatch.setattr(watchlist_scanner_module, "get_primary_source", lambda: "deezer")
|
||||||
|
monkeypatch.setattr(
|
||||||
|
watchlist_scanner_module,
|
||||||
|
"get_client_for_source",
|
||||||
|
lambda source: {
|
||||||
|
"deezer": deezer_client,
|
||||||
|
"itunes": itunes_client,
|
||||||
|
}.get(source),
|
||||||
|
)
|
||||||
|
|
||||||
|
scanner = _build_scanner({"tracks": {"items": []}}, [])
|
||||||
|
scanner.database.get_similar_artists_missing_fallback_ids = (
|
||||||
|
lambda source_artist_id, fallback_source, profile_id=1: [deezer_artist] if fallback_source == "deezer" else [itunes_artist]
|
||||||
|
)
|
||||||
|
|
||||||
|
update_calls = []
|
||||||
|
scanner.database.update_similar_artist_deezer_id = lambda similar_artist_id, deezer_id: update_calls.append(("deezer", similar_artist_id, deezer_id)) or True
|
||||||
|
scanner.database.update_similar_artist_itunes_id = lambda similar_artist_id, itunes_id: update_calls.append(("itunes", similar_artist_id, itunes_id)) or True
|
||||||
|
|
||||||
|
count = scanner._backfill_similar_artists_fallback_ids("source-artist", profile_id=7)
|
||||||
|
|
||||||
|
assert count == 2
|
||||||
|
assert update_calls == [
|
||||||
|
("deezer", 11, "deezer-deezer-artist"),
|
||||||
|
("itunes", 22, "itunes-itunes-artist"),
|
||||||
|
]
|
||||||
|
assert [call[0] for call in deezer_client.search_calls] == ["Deezer Artist"]
|
||||||
|
assert [call[0] for call in itunes_client.search_calls] == ["iTunes Artist"]
|
||||||
|
|
||||||
|
|
||||||
def test_scan_watchlist_profile_loads_artists_and_applies_overrides(monkeypatch):
|
def test_scan_watchlist_profile_loads_artists_and_applies_overrides(monkeypatch):
|
||||||
artist = _build_artist()
|
artist = _build_artist()
|
||||||
scanner = _build_scanner({"tracks": {"items": []}}, [artist])
|
scanner = _build_scanner({"tracks": {"items": []}}, [artist])
|
||||||
|
|
@ -352,7 +400,7 @@ def test_scan_watchlist_artists_scans_tracks_and_updates_state(monkeypatch):
|
||||||
monkeypatch.setattr(scanner, "add_track_to_wishlist", lambda *_args, **_kwargs: True)
|
monkeypatch.setattr(scanner, "add_track_to_wishlist", lambda *_args, **_kwargs: True)
|
||||||
monkeypatch.setattr(scanner, "update_artist_scan_timestamp", lambda *_args, **_kwargs: True)
|
monkeypatch.setattr(scanner, "update_artist_scan_timestamp", lambda *_args, **_kwargs: True)
|
||||||
monkeypatch.setattr(scanner, "update_similar_artists", lambda *_args, **_kwargs: True)
|
monkeypatch.setattr(scanner, "update_similar_artists", lambda *_args, **_kwargs: True)
|
||||||
monkeypatch.setattr(scanner, "_backfill_similar_artists_itunes_ids", lambda *_args, **_kwargs: 0)
|
monkeypatch.setattr(scanner, "_backfill_similar_artists_fallback_ids", lambda *_args, **_kwargs: 0)
|
||||||
|
|
||||||
scan_state = {}
|
scan_state = {}
|
||||||
results = scanner.scan_watchlist_artists([artist], scan_state=scan_state)
|
results = scanner.scan_watchlist_artists([artist], scan_state=scan_state)
|
||||||
|
|
@ -412,7 +460,7 @@ def test_scan_watchlist_artists_skips_placeholder_tracklists(monkeypatch):
|
||||||
monkeypatch.setattr(scanner, "add_track_to_wishlist", lambda *args, **kwargs: add_calls.append((args, kwargs)) or True)
|
monkeypatch.setattr(scanner, "add_track_to_wishlist", lambda *args, **kwargs: add_calls.append((args, kwargs)) or True)
|
||||||
monkeypatch.setattr(scanner, "update_artist_scan_timestamp", lambda *_args, **_kwargs: True)
|
monkeypatch.setattr(scanner, "update_artist_scan_timestamp", lambda *_args, **_kwargs: True)
|
||||||
monkeypatch.setattr(scanner, "update_similar_artists", lambda *_args, **_kwargs: True)
|
monkeypatch.setattr(scanner, "update_similar_artists", lambda *_args, **_kwargs: True)
|
||||||
monkeypatch.setattr(scanner, "_backfill_similar_artists_itunes_ids", lambda *_args, **_kwargs: 0)
|
monkeypatch.setattr(scanner, "_backfill_similar_artists_fallback_ids", lambda *_args, **_kwargs: 0)
|
||||||
|
|
||||||
scan_state = {}
|
scan_state = {}
|
||||||
results = scanner.scan_watchlist_artists([artist], scan_state=scan_state)
|
results = scanner.scan_watchlist_artists([artist], scan_state=scan_state)
|
||||||
|
|
@ -451,7 +499,7 @@ def test_scan_watchlist_artists_honors_cancel_check(monkeypatch):
|
||||||
monkeypatch.setattr(scanner, "add_track_to_wishlist", lambda *_args, **_kwargs: False)
|
monkeypatch.setattr(scanner, "add_track_to_wishlist", lambda *_args, **_kwargs: False)
|
||||||
monkeypatch.setattr(scanner, "update_artist_scan_timestamp", lambda *_args, **_kwargs: True)
|
monkeypatch.setattr(scanner, "update_artist_scan_timestamp", lambda *_args, **_kwargs: True)
|
||||||
monkeypatch.setattr(scanner, "update_similar_artists", lambda *_args, **_kwargs: True)
|
monkeypatch.setattr(scanner, "update_similar_artists", lambda *_args, **_kwargs: True)
|
||||||
monkeypatch.setattr(scanner, "_backfill_similar_artists_itunes_ids", lambda *_args, **_kwargs: 0)
|
monkeypatch.setattr(scanner, "_backfill_similar_artists_fallback_ids", lambda *_args, **_kwargs: 0)
|
||||||
|
|
||||||
cancels = iter([False, True])
|
cancels = iter([False, True])
|
||||||
scan_state = {}
|
scan_state = {}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue