#863: move YouTube artist recovery out of the (synchronous) parse into the async discovery worker — parse is fast again, no 120s timeout risk
This commit is contained in:
parent
0093af89d2
commit
c72e83bc2f
2 changed files with 39 additions and 49 deletions
|
|
@ -52,6 +52,10 @@ class YoutubeDiscoveryDeps:
|
||||||
build_discovery_wing_it_stub: Callable
|
build_discovery_wing_it_stub: Callable
|
||||||
get_database: Callable[[], Any]
|
get_database: Callable[[], Any]
|
||||||
add_activity_item: Callable
|
add_activity_item: Callable
|
||||||
|
# Recover a YouTube track's artist from its own video page when flat playlist
|
||||||
|
# extraction left it "Unknown Artist" (#863). Takes a video id, returns a raw
|
||||||
|
# artist string or ''. Optional — discovery still works without it.
|
||||||
|
recover_youtube_artist: Callable[[str], str] = None
|
||||||
|
|
||||||
|
|
||||||
def run_youtube_discovery_worker(url_hash, deps: YoutubeDiscoveryDeps):
|
def run_youtube_discovery_worker(url_hash, deps: YoutubeDiscoveryDeps):
|
||||||
|
|
@ -94,6 +98,23 @@ def run_youtube_discovery_worker(url_hash, deps: YoutubeDiscoveryDeps):
|
||||||
cleaned_title = track['name']
|
cleaned_title = track['name']
|
||||||
cleaned_artist = track['artists'][0] if track['artists'] else 'Unknown Artist'
|
cleaned_artist = track['artists'][0] if track['artists'] else 'Unknown Artist'
|
||||||
|
|
||||||
|
# Recover the artist from the track's own video page if flat
|
||||||
|
# playlist extraction left it Unknown (#863). Done here, in the
|
||||||
|
# background worker, rather than in the parse request (which would
|
||||||
|
# block for minutes on a big playlist). Per-track cost is hidden
|
||||||
|
# behind the discovery progress bar; the recovered artist makes the
|
||||||
|
# match below actually find the song.
|
||||||
|
if (cleaned_artist == 'Unknown Artist' and deps.recover_youtube_artist
|
||||||
|
and track.get('id')):
|
||||||
|
try:
|
||||||
|
_rec = deps.recover_youtube_artist(track['id'])
|
||||||
|
except Exception as _rec_err:
|
||||||
|
logger.debug(f"Artist recovery failed for {track.get('id')}: {_rec_err}")
|
||||||
|
_rec = ''
|
||||||
|
if _rec and _rec != 'Unknown Artist':
|
||||||
|
cleaned_artist = _rec
|
||||||
|
track['artists'] = [_rec] # persist so retries/UI see it
|
||||||
|
|
||||||
logger.info(f"Searching {discovery_source} for: '{cleaned_artist}' - '{cleaned_title}'")
|
logger.info(f"Searching {discovery_source} for: '{cleaned_artist}' - '{cleaned_title}'")
|
||||||
|
|
||||||
# Check discovery cache first
|
# Check discovery cache first
|
||||||
|
|
|
||||||
|
|
@ -14776,6 +14776,16 @@ def _fetch_youtube_video_artist(video_id, cookie_opts):
|
||||||
return (info.get('uploader') or info.get('channel') or '').strip()
|
return (info.get('uploader') or info.get('channel') or '').strip()
|
||||||
|
|
||||||
|
|
||||||
|
def _recover_youtube_artist_cleaned(video_id):
|
||||||
|
"""Fetch + clean a YouTube track's artist from its video page (#863).
|
||||||
|
Used by the async discovery worker for tracks flat extraction left Unknown."""
|
||||||
|
raw = _fetch_youtube_video_artist(video_id, _youtube_cookie_opts())
|
||||||
|
if not raw:
|
||||||
|
return ''
|
||||||
|
cleaned = clean_youtube_artist(raw)
|
||||||
|
return cleaned if cleaned and cleaned != 'Unknown Artist' else ''
|
||||||
|
|
||||||
|
|
||||||
def parse_youtube_playlist(url):
|
def parse_youtube_playlist(url):
|
||||||
"""
|
"""
|
||||||
Parse a YouTube Music playlist URL and extract track information using yt-dlp
|
Parse a YouTube Music playlist URL and extract track information using yt-dlp
|
||||||
|
|
@ -14848,55 +14858,13 @@ def parse_youtube_playlist(url):
|
||||||
|
|
||||||
tracks.append(track_data)
|
tracks.append(track_data)
|
||||||
|
|
||||||
# ARTIST RECOVERY (#863): current yt-dlp flat extraction returns ONLY
|
# NOTE: current yt-dlp flat extraction returns ONLY the title per entry
|
||||||
# the title per entry — no uploader/artist — so tracks whose title
|
# (no uploader/artist), so tracks whose title isn't "Artist - Title"
|
||||||
# isn't "Artist - Title" came out "Unknown Artist" and discovery
|
# land here as "Unknown Artist". The per-video artist recovery does NOT
|
||||||
# couldn't match them. Recover the artist from each unresolved track's
|
# run here — it would block this request for minutes on a big playlist
|
||||||
# OWN video page (uploader/channel). Bounded by a wall-clock budget so
|
# and risk the 120s worker timeout. It runs in the async discovery
|
||||||
# a large playlist can't exceed the 120s gunicorn worker timeout;
|
# worker instead (which already iterates every track with a progress
|
||||||
# whatever isn't reached stays for MusicBrainz/Spotify discovery.
|
# bar). See run_youtube_discovery_worker / recover_youtube_artist (#863).
|
||||||
unresolved = [t for t in tracks
|
|
||||||
if t.get('id') and (not t['artists'] or t['artists'][0] == 'Unknown Artist')]
|
|
||||||
if unresolved:
|
|
||||||
cookie_opts = _youtube_cookie_opts()
|
|
||||||
deadline = time.time() + 75 # leave headroom under the 120s worker timeout
|
|
||||||
recovered = 0
|
|
||||||
# Per-video lookups are several seconds each, so doing them
|
|
||||||
# sequentially only covered ~a dozen tracks before the budget ran
|
|
||||||
# out (the back half of a playlist stayed "Unknown Artist"). Run a
|
|
||||||
# small pool concurrently to cover the whole playlist within the
|
|
||||||
# same wall-clock budget, without hammering YouTube.
|
|
||||||
from concurrent.futures import (ThreadPoolExecutor, as_completed,
|
|
||||||
TimeoutError as _FuturesTimeout)
|
|
||||||
ex = ThreadPoolExecutor(max_workers=5, thread_name_prefix="yt_artist")
|
|
||||||
fut_to_track = {ex.submit(_fetch_youtube_video_artist, t['id'], cookie_opts): t
|
|
||||||
for t in unresolved}
|
|
||||||
try:
|
|
||||||
for fut in as_completed(fut_to_track, timeout=max(1.0, deadline - time.time())):
|
|
||||||
t = fut_to_track[fut]
|
|
||||||
try:
|
|
||||||
raw_artist = fut.result()
|
|
||||||
except Exception:
|
|
||||||
raw_artist = ''
|
|
||||||
if raw_artist:
|
|
||||||
ca = clean_youtube_artist(raw_artist)
|
|
||||||
if ca and ca != 'Unknown Artist':
|
|
||||||
t['artists'] = [ca]
|
|
||||||
t['name'] = clean_youtube_track_title(t.get('name') or '', ca)
|
|
||||||
t['raw_artist'] = raw_artist
|
|
||||||
recovered += 1
|
|
||||||
except _FuturesTimeout:
|
|
||||||
logger.warning(
|
|
||||||
f"[YT Parse] Artist-recovery budget hit; recovered {recovered}/{len(unresolved)}, "
|
|
||||||
"rest left for discovery to resolve"
|
|
||||||
)
|
|
||||||
finally:
|
|
||||||
# Don't block the response waiting on in-flight lookups past
|
|
||||||
# the budget — cancel what hasn't started, let the rest finish
|
|
||||||
# in the background.
|
|
||||||
ex.shutdown(wait=False, cancel_futures=True)
|
|
||||||
if recovered:
|
|
||||||
logger.info(f"[YT Parse] Recovered artist for {recovered}/{len(unresolved)} unresolved tracks")
|
|
||||||
|
|
||||||
# Create playlist object matching GUI structure
|
# Create playlist object matching GUI structure
|
||||||
playlist_data = {
|
playlist_data = {
|
||||||
|
|
@ -24748,6 +24716,7 @@ def _build_youtube_discovery_deps():
|
||||||
build_discovery_wing_it_stub=_build_discovery_wing_it_stub,
|
build_discovery_wing_it_stub=_build_discovery_wing_it_stub,
|
||||||
get_database=get_database,
|
get_database=get_database,
|
||||||
add_activity_item=add_activity_item,
|
add_activity_item=add_activity_item,
|
||||||
|
recover_youtube_artist=_recover_youtube_artist_cleaned,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue