video enrichment: distinguish transient 'error' from 'not_found' (match music)
A failed lookup CALL (network/429/5xx/timeout, or an expired TVDB token) was recorded as 'not_found' — permanently logging a transient blip as 'no match' and parking the item for retry_days. Now mirrors the music workers' proven pattern: - New 'error' status, distinct from 'not_found'; enrichment_next retries BOTH after retry_days, so errors recover and the queue still advances (no poison loop). breakdown/unmatched/retry-all and the modal account for it (shown with the outstanding/pending bucket). - TMDB/TVDB clients raise on non-200 (429/5xx) so the worker records 'error', not a false not_found. - TVDB re-authenticates once on a 401 (expired token) instead of failing every match for the rest of the run. Seam tests: error!=not_found, error retried after window, 429 raises, TVDB token refresh, UI accounts for errors.
This commit is contained in:
parent
fc68a6e741
commit
a483219746
6 changed files with 139 additions and 28 deletions
|
|
@ -62,6 +62,10 @@ class TMDBClient:
|
|||
if year:
|
||||
params["year" if kind == "movie" else "first_air_date_year"] = year
|
||||
resp = requests.get(self.BASE + path, params=params, timeout=15)
|
||||
# A non-200 (429 rate-limit, 5xx, timeout-as-error) is a FAILED call,
|
||||
# not "no match" — raise so the worker records 'error' (retried later)
|
||||
# instead of burning the item to 'not_found'.
|
||||
resp.raise_for_status()
|
||||
results = (resp.json() or {}).get("results") or []
|
||||
if not results:
|
||||
return None
|
||||
|
|
@ -112,30 +116,41 @@ class TVDBClient:
|
|||
logger.exception("TVDB test failed")
|
||||
return False, "Could not reach TVDB"
|
||||
|
||||
def _auth(self):
|
||||
if self._token:
|
||||
def _auth(self, force=False):
|
||||
if self._token and not force:
|
||||
return self._token
|
||||
import requests
|
||||
self._token = None
|
||||
r = requests.post(self.BASE + "/login", json={"apikey": self.api_key}, timeout=15).json() or {}
|
||||
self._token = (r.get("data") or {}).get("token")
|
||||
return self._token
|
||||
|
||||
def match(self, kind, title, year, known_id=None):
|
||||
if kind != "show" or not self.api_key:
|
||||
return None
|
||||
def _authed_get(self, path, params=None):
|
||||
"""GET with the bearer token, transparently re-authenticating once if the
|
||||
cached token has expired (401). Raises on any other non-200 so the worker
|
||||
records 'error' rather than a false 'not_found'."""
|
||||
import requests
|
||||
token = self._auth()
|
||||
if not token:
|
||||
return None
|
||||
headers = {"Authorization": "Bearer " + token}
|
||||
r = requests.get(self.BASE + path, headers={"Authorization": "Bearer " + token},
|
||||
params=params, timeout=15)
|
||||
if r.status_code == 401 and self._auth(force=True): # token expired → refresh once
|
||||
r = requests.get(self.BASE + path, headers={"Authorization": "Bearer " + self._token},
|
||||
params=params, timeout=15)
|
||||
r.raise_for_status()
|
||||
return r.json() or {}
|
||||
|
||||
def match(self, kind, title, year, known_id=None):
|
||||
if kind != "show" or not self.api_key:
|
||||
return None
|
||||
tvdb_id = _int(known_id)
|
||||
meta = {}
|
||||
if tvdb_id is None:
|
||||
if not title:
|
||||
return None
|
||||
r = requests.get(self.BASE + "/search", headers=headers,
|
||||
params={"query": title, "type": "series"}, timeout=15).json() or {}
|
||||
results = r.get("data") or []
|
||||
r = self._authed_get("/search", {"query": title, "type": "series"})
|
||||
results = (r or {}).get("data") or []
|
||||
if not results:
|
||||
return None
|
||||
top = results[0]
|
||||
|
|
@ -144,9 +159,8 @@ class TVDBClient:
|
|||
else:
|
||||
# Known id from the server → fetch the series directly for its overview.
|
||||
try:
|
||||
dr = requests.get(self.BASE + "/series/" + str(tvdb_id),
|
||||
headers=headers, timeout=15).json() or {}
|
||||
meta["overview"] = (dr.get("data") or {}).get("overview")
|
||||
dr = self._authed_get("/series/" + str(tvdb_id))
|
||||
meta["overview"] = ((dr or {}).get("data") or {}).get("overview")
|
||||
except Exception:
|
||||
logger.exception("TVDB details fetch failed for %s", title or tvdb_id)
|
||||
if tvdb_id is None:
|
||||
|
|
|
|||
|
|
@ -110,7 +110,10 @@ class VideoEnrichmentWorker:
|
|||
except Exception:
|
||||
logger.exception("video enrichment %s match failed for %s", self.service, item["title"])
|
||||
self.stats["errors"] += 1
|
||||
self.db.enrichment_apply(self.service, item["kind"], item["id"], matched=False)
|
||||
# The CALL failed (network/rate-limit/timeout) — record 'error', NOT
|
||||
# 'not_found', so a transient blip isn't permanently logged as "no
|
||||
# match". enrichment_next retries 'error' items after retry_days.
|
||||
self.db.enrichment_apply(self.service, item["kind"], item["id"], matched=False, error=True)
|
||||
return True
|
||||
if result and result.get("id"):
|
||||
self.db.enrichment_apply(self.service, item["kind"], item["id"], matched=True,
|
||||
|
|
@ -124,12 +127,14 @@ class VideoEnrichmentWorker:
|
|||
# ── status (same shape the music enrichment API returns) ──────────────────
|
||||
def get_stats(self) -> dict:
|
||||
breakdown = self.db.enrichment_breakdown(self.service)
|
||||
pending = sum(b["pending"] for b in breakdown.values())
|
||||
# Errored items are outstanding (retried later), so they count as pending
|
||||
# work — the worker isn't "Complete" while any remain.
|
||||
pending = sum(b["pending"] + b.get("errors", 0) for b in breakdown.values())
|
||||
running = self.running and not self.paused and self.enabled
|
||||
idle = running and pending == 0 and self.current_item is None
|
||||
progress = {}
|
||||
for kind, b in breakdown.items():
|
||||
total = b["matched"] + b["not_found"] + b["pending"]
|
||||
total = b["matched"] + b["not_found"] + b.get("errors", 0) + b["pending"]
|
||||
done = b["matched"] + b["not_found"]
|
||||
progress[kind] = {"matched": b["matched"], "total": total,
|
||||
"percent": round(done / total * 100) if total else 0}
|
||||
|
|
|
|||
|
|
@ -170,7 +170,8 @@ class VideoDatabase:
|
|||
return _row(row, kind, idc)
|
||||
for kind, (tbl, idc, sc, ac) in kinds.items():
|
||||
row = conn.execute(
|
||||
f"SELECT id, title, year, {idc} FROM {tbl} WHERE {sc}='not_found' "
|
||||
f"SELECT id, title, year, {idc} FROM {tbl} "
|
||||
f"WHERE {sc} IN ('not_found','error') "
|
||||
f"AND ({ac} IS NULL OR {ac} < ?) ORDER BY {ac} LIMIT 1", (cutoff,)).fetchone()
|
||||
if row:
|
||||
return _row(row, kind, idc)
|
||||
|
|
@ -179,14 +180,22 @@ class VideoDatabase:
|
|||
conn.close()
|
||||
|
||||
def enrichment_apply(self, service: str, kind: str, item_id: int, matched: bool,
|
||||
external_id=None, metadata: dict | None = None) -> None:
|
||||
external_id=None, metadata: dict | None = None,
|
||||
error: bool = False) -> None:
|
||||
"""Record a match result: set match_status + last_attempted, the external
|
||||
id (when matched), and any whitelisted metadata columns."""
|
||||
id (when matched), and any whitelisted metadata columns.
|
||||
|
||||
Status is one of 'matched' / 'not_found' / 'error'. 'error' means the
|
||||
lookup CALL failed (network/rate-limit/timeout) — distinct from a genuine
|
||||
'not_found' so a transient blip isn't permanently recorded as "no match"
|
||||
(mirrors the music workers). Both 'not_found' and 'error' are retried by
|
||||
enrichment_next after retry_days."""
|
||||
spec = _ENRICH.get(service, {}).get(kind)
|
||||
if not spec:
|
||||
return
|
||||
tbl, idc, sc, ac = spec
|
||||
allowed = _ENRICH_META_COLS.get(tbl, set())
|
||||
status = "matched" if matched else "error" if error else "not_found"
|
||||
# On legacy DBs tmdb_id/tvdb_id may still carry a UNIQUE index; if a match
|
||||
# would collide with another row's id we drop the id columns and keep the
|
||||
# existing (authoritative) id, still recording status + metadata.
|
||||
|
|
@ -194,7 +203,7 @@ class VideoDatabase:
|
|||
|
||||
def build(include_ids):
|
||||
sets = [f"{sc}=?", f"{ac}=CURRENT_TIMESTAMP"]
|
||||
params = ["matched" if matched else "not_found"]
|
||||
params = [status]
|
||||
if matched and external_id is not None and include_ids:
|
||||
sets.append(f"{idc}=?")
|
||||
params.append(external_id)
|
||||
|
|
@ -231,6 +240,7 @@ class VideoDatabase:
|
|||
out[kind] = {
|
||||
"matched": conn.execute(f"SELECT COUNT(*) FROM {tbl} WHERE {sc}='matched'").fetchone()[0],
|
||||
"not_found": conn.execute(f"SELECT COUNT(*) FROM {tbl} WHERE {sc}='not_found'").fetchone()[0],
|
||||
"errors": conn.execute(f"SELECT COUNT(*) FROM {tbl} WHERE {sc}='error'").fetchone()[0],
|
||||
"pending": conn.execute(f"SELECT COUNT(*) FROM {tbl} WHERE {sc} IS NULL").fetchone()[0],
|
||||
}
|
||||
return out
|
||||
|
|
@ -247,7 +257,7 @@ class VideoDatabase:
|
|||
if status == "pending":
|
||||
where.append(f"{sc} IS NULL")
|
||||
elif status == "unmatched":
|
||||
where.append(f"({sc} IS NULL OR {sc}='not_found')")
|
||||
where.append(f"({sc} IS NULL OR {sc} IN ('not_found','error'))")
|
||||
else:
|
||||
where.append(f"{sc}='not_found'")
|
||||
if search:
|
||||
|
|
@ -282,7 +292,8 @@ class VideoDatabase:
|
|||
if scope == "item" and item_id is not None:
|
||||
cur = conn.execute(f"UPDATE {tbl} SET {sc}=NULL, {ac}=NULL WHERE id=?", (item_id,))
|
||||
else:
|
||||
cur = conn.execute(f"UPDATE {tbl} SET {sc}=NULL, {ac}=NULL WHERE {sc}='not_found'")
|
||||
cur = conn.execute(
|
||||
f"UPDATE {tbl} SET {sc}=NULL, {ac}=NULL WHERE {sc} IN ('not_found','error')")
|
||||
conn.commit()
|
||||
return cur.rowcount
|
||||
finally:
|
||||
|
|
|
|||
|
|
@ -387,13 +387,27 @@ def test_enrichment_breakdown_unmatched_retry(db):
|
|||
b = db.upsert_movie("plex", {"server_id": "m2", "title": "B"})
|
||||
db.enrichment_apply("tmdb", "movie", a, matched=True, external_id=1)
|
||||
db.enrichment_apply("tmdb", "movie", b, matched=False)
|
||||
assert db.enrichment_breakdown("tmdb")["movie"] == {"matched": 1, "not_found": 1, "pending": 0}
|
||||
assert db.enrichment_breakdown("tmdb")["movie"] == {"matched": 1, "not_found": 1, "errors": 0, "pending": 0}
|
||||
un = db.enrichment_unmatched("tmdb", "movie", status="not_found")
|
||||
assert [i["title"] for i in un["items"]] == ["B"] and un["total"] == 1
|
||||
assert db.enrichment_retry("tmdb", "movie", scope="failed") == 1
|
||||
assert db.enrichment_breakdown("tmdb")["movie"]["pending"] == 1
|
||||
|
||||
|
||||
def test_error_status_is_distinct_and_retryable_in_ui(db):
|
||||
a = db.upsert_movie("plex", {"server_id": "m1", "title": "A"})
|
||||
db.enrichment_apply("tmdb", "movie", a, matched=False, error=True)
|
||||
bd = db.enrichment_breakdown("tmdb")["movie"]
|
||||
assert bd == {"matched": 0, "not_found": 0, "errors": 1, "pending": 0}
|
||||
# Errored items surface in the modal's "unmatched" list so they can be retried.
|
||||
assert db.enrichment_unmatched("tmdb", "movie", status="unmatched")["total"] == 1
|
||||
# ...but NOT in the strict 'not_found'-only view.
|
||||
assert db.enrichment_unmatched("tmdb", "movie", status="not_found")["total"] == 0
|
||||
# "Retry all failed" re-queues errors too (back to pending/NULL).
|
||||
assert db.enrichment_retry("tmdb", "movie", scope="failed") == 1
|
||||
assert db.enrichment_breakdown("tmdb")["movie"]["pending"] == 1
|
||||
|
||||
|
||||
def test_tvdb_is_shows_only(db):
|
||||
# TVDB enriches shows, never movies. The breakdown must not advertise a
|
||||
# movie bucket, and asking for tvdb movies returns empty (never garbage) —
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ import pytest
|
|||
from database.video_database import VideoDatabase
|
||||
from core.video.enrichment.worker import VideoEnrichmentWorker
|
||||
from core.video.enrichment.engine import VideoEnrichmentEngine
|
||||
from core.video.enrichment.clients import TMDBClient
|
||||
from core.video.enrichment.clients import TMDBClient, TVDBClient
|
||||
|
||||
|
||||
class FakeClient:
|
||||
|
|
@ -83,6 +83,52 @@ def test_tmdb_client_with_known_id_skips_the_search(monkeypatch):
|
|||
assert not any("/search/" in u for u in urls)
|
||||
|
||||
|
||||
def test_tmdb_client_raises_on_rate_limit(monkeypatch):
|
||||
# A 429 must raise (→ worker records 'error'), not silently return no-match.
|
||||
class _Resp429:
|
||||
status_code = 429
|
||||
def raise_for_status(self):
|
||||
raise RuntimeError("429 Too Many Requests")
|
||||
def json(self):
|
||||
return {}
|
||||
|
||||
fake = types.SimpleNamespace(get=lambda url, **kw: _Resp429())
|
||||
monkeypatch.setitem(sys.modules, "requests", fake)
|
||||
with pytest.raises(Exception):
|
||||
TMDBClient("KEY").match("movie", "Dune", 2021)
|
||||
|
||||
|
||||
def test_tvdb_client_refreshes_expired_token(monkeypatch):
|
||||
# Cached token returns 401 → the client must re-login once and retry, not fail.
|
||||
logins = []
|
||||
state = {"calls": 0}
|
||||
|
||||
class _Resp:
|
||||
def __init__(self, code, body):
|
||||
self.status_code = code
|
||||
self._body = body
|
||||
def raise_for_status(self):
|
||||
if self.status_code >= 400:
|
||||
raise RuntimeError("HTTP " + str(self.status_code))
|
||||
def json(self):
|
||||
return self._body
|
||||
|
||||
def fake_post(url, **kw):
|
||||
logins.append(url)
|
||||
return _Resp(200, {"data": {"token": "tok%d" % len(logins)}})
|
||||
|
||||
def fake_get(url, **kw):
|
||||
state["calls"] += 1
|
||||
if state["calls"] == 1:
|
||||
return _Resp(401, {}) # stale token rejected
|
||||
return _Resp(200, {"data": [{"tvdb_id": 77, "overview": "O"}]})
|
||||
|
||||
monkeypatch.setitem(sys.modules, "requests", types.SimpleNamespace(get=fake_get, post=fake_post))
|
||||
res = TVDBClient("KEY").match("show", "Some Show", 2020)
|
||||
assert res["id"] == 77
|
||||
assert len(logins) == 2 # initial login + one refresh after the 401
|
||||
|
||||
|
||||
def test_worker_process_one_not_found(db):
|
||||
db.upsert_movie("plex", {"server_id": "m1", "title": "X"})
|
||||
w = VideoEnrichmentWorker(db, "tmdb", FakeClient(None))
|
||||
|
|
@ -96,7 +142,10 @@ def test_worker_process_one_no_items_returns_false(db):
|
|||
assert VideoEnrichmentWorker(db, "tmdb", FakeClient(None)).process_one() is False
|
||||
|
||||
|
||||
def test_worker_match_exception_marks_not_found_and_counts_error(db):
|
||||
def test_worker_match_exception_marks_error_not_notfound(db):
|
||||
# A failed CALL must be recorded as 'error' (a transient failure), NOT
|
||||
# 'not_found' — otherwise a network blip permanently logs the item as "no
|
||||
# match" and it won't retry for retry_days. Mirrors the music workers.
|
||||
db.upsert_movie("plex", {"server_id": "m1", "title": "X"})
|
||||
|
||||
class Boom:
|
||||
|
|
@ -107,7 +156,22 @@ def test_worker_match_exception_marks_not_found_and_counts_error(db):
|
|||
assert w.process_one() is True # doesn't crash the loop
|
||||
assert w.stats["errors"] == 1
|
||||
with db.connect() as c:
|
||||
assert c.execute("SELECT tmdb_match_status FROM movies").fetchone()["tmdb_match_status"] == "not_found"
|
||||
assert c.execute("SELECT tmdb_match_status FROM movies").fetchone()["tmdb_match_status"] == "error"
|
||||
|
||||
|
||||
def test_errored_item_is_retried_after_retry_days(db):
|
||||
# An 'error' row is re-queued by enrichment_next once it's older than the
|
||||
# retry window (just like 'not_found'), so transient failures recover.
|
||||
mid = db.upsert_movie("plex", {"server_id": "m1", "title": "X"})
|
||||
db.enrichment_apply("tmdb", "movie", mid, matched=False, error=True)
|
||||
# Just attempted → still inside the retry window → not yet due.
|
||||
assert db.enrichment_next("tmdb", retry_days=30) is None
|
||||
# Backdate the attempt → now older than the window → re-queued for retry.
|
||||
with db.connect() as c:
|
||||
c.execute("UPDATE movies SET tmdb_last_attempted='2000-01-01 00:00:00' WHERE id=?", (mid,))
|
||||
c.commit()
|
||||
again = db.enrichment_next("tmdb", retry_days=30)
|
||||
assert again is not None and again["id"] == mid
|
||||
|
||||
|
||||
def test_worker_get_stats_shape(db):
|
||||
|
|
|
|||
|
|
@ -168,8 +168,11 @@
|
|||
var kinds = Object.keys(bd);
|
||||
host.innerHTML = kinds.map(function (e) {
|
||||
var d = bd[e] || {};
|
||||
var total = (d.matched || 0) + (d.not_found || 0) + (d.pending || 0);
|
||||
var matched = d.matched || 0, nf = d.not_found || 0, pend = d.pending || 0;
|
||||
// Errored items are outstanding (retried later) — show them with the
|
||||
// pending bucket so the bar/total stay honest.
|
||||
var matched = d.matched || 0, nf = d.not_found || 0;
|
||||
var pend = (d.pending || 0) + (d.errors || 0);
|
||||
var total = matched + nf + pend;
|
||||
var pct = total ? Math.round(matched / total * 100) : 0;
|
||||
var seg = function (n) { return total ? (n / total) * 100 : 0; };
|
||||
var active = e === state.kind ? ' em-card--current' : '';
|
||||
|
|
@ -189,7 +192,7 @@
|
|||
var m = 0, t = 0;
|
||||
kinds.forEach(function (e) {
|
||||
var d = bd[e] || {}; m += d.matched || 0;
|
||||
t += (d.matched || 0) + (d.not_found || 0) + (d.pending || 0);
|
||||
t += (d.matched || 0) + (d.not_found || 0) + (d.pending || 0) + (d.errors || 0);
|
||||
});
|
||||
overall.innerHTML = t ? '<strong>' + (t ? Math.round(m / t * 100) : 0) + '%</strong> matched · '
|
||||
+ m + ' of ' + t : '';
|
||||
|
|
|
|||
Loading…
Reference in a new issue