video: OMDb worker survives a bad/expired API key gracefully
The log flood you saw was the OMDb worker hitting a 401 (invalid key) on every owned title: it logged a full traceback per item AND marked each one ratings_synced=1 — which would've stopped them ever retrying once the key was fixed. Root-cause fixes: - OMDBClient.ratings raises a distinct OMDbAuthError on 401 / 'Invalid API key!' (vs a transient error vs a genuine no-data 200). - Worker: on an auth error it PAUSES (transient, not persisted) with a reason note + one warning, instead of churning the whole library; the item is NOT marked synced. Transient errors no longer burn items either — they back off and pause after 3 in a row. Only a genuine 'no data' marks an item synced. Warnings are concise (no per-item tracebacks). get_stats exposes the pause 'note'. - Fixing the key auto-recovers: saving a new/changed OMDb key re-queues every still-unrated title (resets the wrongly-burned ones), and the engine rebuild un-pauses the worker. Seam tests: bad-key pause-without-burn, transient keep-item, ratings() raises on 401, key-change re-queues unrated. 227 video-suite tests pass.
This commit is contained in:
parent
df889c0c6e
commit
d0c8aa9338
5 changed files with 113 additions and 12 deletions
|
|
@ -58,7 +58,17 @@ def register_routes(bp):
|
|||
if "tvdb_api_key" in body:
|
||||
db.set_setting("tvdb_api_key", body.get("tvdb_api_key") or "")
|
||||
if "omdb_api_key" in body:
|
||||
db.set_setting("omdb_api_key", body.get("omdb_api_key") or "")
|
||||
new_key = body.get("omdb_api_key") or ""
|
||||
changed = new_key != (db.get_setting("omdb_api_key") or "")
|
||||
db.set_setting("omdb_api_key", new_key)
|
||||
# A new/changed OMDb key → re-try every title that still has no rating
|
||||
# (covers items wrongly marked 'synced' during a prior bad-key run).
|
||||
if new_key and changed:
|
||||
try:
|
||||
for kind in ("movie", "show"):
|
||||
db.enrichment_retry("omdb", kind, scope="failed")
|
||||
except Exception:
|
||||
logger.exception("video enrichment: omdb re-try reset failed")
|
||||
try:
|
||||
from core.video.enrichment.engine import rebuild_video_enrichment_engine
|
||||
rebuild_video_enrichment_engine()
|
||||
|
|
|
|||
|
|
@ -481,6 +481,12 @@ class TVDBClient:
|
|||
return {"id": tvdb_id, "metadata": {k: v for k, v in meta.items() if v}}
|
||||
|
||||
|
||||
class OMDbAuthError(Exception):
|
||||
"""OMDb rejected the API key (HTTP 401 / 'Invalid API key!'). Distinct from a
|
||||
transient error or a genuine 'no rating' so the worker can pause instead of
|
||||
churning the whole library on a bad key."""
|
||||
|
||||
|
||||
class OMDBClient:
|
||||
"""Ratings provider — IMDb / Rotten Tomatoes / Metacritic by imdb_id. Not a
|
||||
matcher (we already have the id), so it's used as a ratings backfill, not a
|
||||
|
|
@ -515,10 +521,16 @@ class OMDBClient:
|
|||
return None
|
||||
import requests
|
||||
r = requests.get(self.BASE, params={"apikey": self.api_key, "i": imdb_id}, timeout=12)
|
||||
# A bad/expired key is a 401 (sometimes a 200 with "Invalid API key!") — a
|
||||
# config problem that affects EVERY item, so flag it distinctly.
|
||||
if r.status_code == 401:
|
||||
raise OMDbAuthError("OMDb rejected the API key (HTTP 401)")
|
||||
r.raise_for_status()
|
||||
d = r.json() or {}
|
||||
if d.get("Response") != "True":
|
||||
return None
|
||||
if "invalid api key" in (d.get("Error") or "").lower():
|
||||
raise OMDbAuthError(d.get("Error") or "Invalid OMDb API key")
|
||||
return None # genuine "no data for this title"
|
||||
out = {}
|
||||
ir = d.get("imdbRating")
|
||||
if ir and ir != "N/A":
|
||||
|
|
|
|||
|
|
@ -36,6 +36,8 @@ class VideoEnrichmentWorker:
|
|||
self._stop = threading.Event()
|
||||
self.current_item = None
|
||||
self.stats = {"matched": 0, "not_found": 0, "errors": 0}
|
||||
self.note = None # a human reason when auto-paused (e.g. bad key)
|
||||
self._rating_errors = 0 # consecutive ratings failures (transient backoff)
|
||||
|
||||
# ── lifecycle ─────────────────────────────────────────────────────────────
|
||||
def start(self):
|
||||
|
|
@ -149,23 +151,45 @@ class VideoEnrichmentWorker:
|
|||
def _process_ratings_one(self) -> bool:
|
||||
"""OMDb worker: fetch IMDb/RT/Metacritic for the next library item that has
|
||||
an imdb_id but no ratings yet."""
|
||||
from .clients import OMDbAuthError
|
||||
item = self.db.ratings_next()
|
||||
if not item:
|
||||
self._rating_errors = 0
|
||||
return False
|
||||
self.current_item = {"type": item["kind"], "name": item["title"]}
|
||||
try:
|
||||
r = self.client.ratings(item["imdb_id"])
|
||||
if r:
|
||||
self.db.apply_ratings(item["kind"], item["id"], r) # marks synced
|
||||
self.stats["matched"] += 1
|
||||
logger.info("Rated %s '%s' -> IMDb %s", item["kind"], item["title"], item["imdb_id"])
|
||||
else:
|
||||
self.db.mark_ratings_synced(item["kind"], item["id"])
|
||||
self.stats["not_found"] += 1
|
||||
except Exception:
|
||||
logger.exception("OMDb ratings fetch failed for '%s'", item["title"])
|
||||
except OMDbAuthError as e:
|
||||
# Bad/expired key affects EVERY item — pause instead of churning the
|
||||
# whole library + flooding the log. Transient pause (not persisted) so
|
||||
# fixing the key (which rebuilds the engine) resumes automatically. The
|
||||
# item is NOT marked synced, so it'll be tried again once the key works.
|
||||
if not self.paused:
|
||||
logger.warning("OMDb rejected the API key — pausing ratings until it's fixed (%s)", e)
|
||||
self.note = "OMDb API key rejected"
|
||||
self.pause(persist=False)
|
||||
return False
|
||||
except Exception as e:
|
||||
# Transient (network / rate-limit / 5xx) — don't burn the item to
|
||||
# 'synced'; back off, and pause after a few in a row so we don't spin.
|
||||
self.stats["errors"] += 1
|
||||
self.db.mark_ratings_synced(item["kind"], item["id"]) # move on (no loop)
|
||||
self._rating_errors = getattr(self, "_rating_errors", 0) + 1
|
||||
logger.warning("OMDb ratings fetch failed for '%s': %s", item["title"], e)
|
||||
if self._rating_errors >= 3:
|
||||
logger.warning("OMDb: pausing ratings after repeated errors")
|
||||
self.note = "OMDb temporarily unavailable"
|
||||
self.pause(persist=False)
|
||||
self._rating_errors = 0
|
||||
return False
|
||||
self._rating_errors = 0
|
||||
self.note = None
|
||||
if r:
|
||||
self.db.apply_ratings(item["kind"], item["id"], r) # marks synced
|
||||
self.stats["matched"] += 1
|
||||
logger.info("Rated %s '%s' -> IMDb %s", item["kind"], item["title"], item["imdb_id"])
|
||||
else:
|
||||
self.db.mark_ratings_synced(item["kind"], item["id"]) # genuine no-data
|
||||
self.stats["not_found"] += 1
|
||||
return True
|
||||
|
||||
def _sync_episodes_once(self) -> bool:
|
||||
|
|
@ -248,6 +272,7 @@ class VideoEnrichmentWorker:
|
|||
"paused": self.paused,
|
||||
"idle": idle,
|
||||
"current_item": self.current_item,
|
||||
"note": self.note,
|
||||
"stats": {**self.stats, "pending": pending},
|
||||
"progress": progress,
|
||||
"breakdown": breakdown,
|
||||
|
|
|
|||
|
|
@ -83,6 +83,18 @@ def test_tmdb_detail_endpoint(tmp_path, monkeypatch):
|
|||
assert client.get("/api/video/tmdb/bogus/1").status_code == 400
|
||||
|
||||
|
||||
def test_omdb_key_change_retries_unrated(tmp_path, monkeypatch):
|
||||
client, videoapi = _make_client(tmp_path)
|
||||
db = videoapi._video_db
|
||||
mid = db.upsert_movie("plex", {"server_id": "m1", "title": "A", "imdb_id": "tt1"})
|
||||
db.apply_ratings("movie", mid, {}) # burned: synced, but no rating
|
||||
assert db.ratings_next() is None # not pending
|
||||
monkeypatch.setattr("core.video.enrichment.engine.rebuild_video_enrichment_engine", lambda: None)
|
||||
resp = client.post("/api/video/enrichment/config", json={"omdb_api_key": "NEWKEY"})
|
||||
assert resp.status_code == 200
|
||||
assert db.ratings_next() is not None # new key → re-queued for rating
|
||||
|
||||
|
||||
def test_show_detail_endpoint(tmp_path):
|
||||
client, videoapi = _make_client(tmp_path)
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -325,6 +325,48 @@ def test_omdb_worker_processes_ratings_queue(db):
|
|||
assert w.process_one() is False
|
||||
|
||||
|
||||
def test_omdb_worker_pauses_on_bad_key_without_burning_items(db):
|
||||
# A rejected key affects every title — the worker must pause (not churn the
|
||||
# whole library) and must NOT mark items synced, so they retry once fixed.
|
||||
from core.video.enrichment.clients import OMDbAuthError
|
||||
db.upsert_movie("plex", {"server_id": "m1", "title": "A", "imdb_id": "tt1"})
|
||||
|
||||
class Omdb:
|
||||
enabled = True
|
||||
def ratings(self, imdb_id): raise OMDbAuthError("401")
|
||||
|
||||
w = VideoEnrichmentWorker(db, "omdb", Omdb())
|
||||
assert w.process_one() is False # backed off, didn't process
|
||||
assert w.paused is True and w.note # paused itself with a reason
|
||||
assert db.ratings_next() is not None # item NOT burned to 'synced'
|
||||
|
||||
|
||||
def test_omdb_worker_keeps_item_on_transient_error(db):
|
||||
db.upsert_movie("plex", {"server_id": "m1", "title": "A", "imdb_id": "tt1"})
|
||||
|
||||
class Omdb:
|
||||
enabled = True
|
||||
def ratings(self, imdb_id): raise RuntimeError("network blip")
|
||||
|
||||
w = VideoEnrichmentWorker(db, "omdb", Omdb())
|
||||
assert w.process_one() is False
|
||||
assert db.ratings_next() is not None # not marked synced → retried later
|
||||
assert w.stats["errors"] == 1 and w.paused is False # one blip doesn't pause
|
||||
|
||||
|
||||
def test_omdb_ratings_raises_on_invalid_key(monkeypatch):
|
||||
from core.video.enrichment.clients import OMDbAuthError
|
||||
|
||||
class _R:
|
||||
status_code = 401
|
||||
text = ""
|
||||
def raise_for_status(self): raise AssertionError("should short-circuit on 401")
|
||||
def json(self): return {}
|
||||
monkeypatch.setitem(sys.modules, "requests", types.SimpleNamespace(get=lambda u, **k: _R()))
|
||||
with pytest.raises(OMDbAuthError):
|
||||
OMDBClient("BADKEY").ratings("tt0111161")
|
||||
|
||||
|
||||
def test_omdb_breakdown_is_ratings_coverage(db):
|
||||
a = db.upsert_movie("plex", {"server_id": "m1", "title": "A", "imdb_id": "tt1"}) # pending
|
||||
db.upsert_movie("plex", {"server_id": "m2", "title": "B", "imdb_id": "tt2"})
|
||||
|
|
|
|||
Loading…
Reference in a new issue