Add artwork self-hosting: local-disk download worker + storage
CI / lint (push) Canceled after 0s
CI / typecheck (push) Canceled after 0s
CI / unit-tests (push) Canceled after 0s
CI / integration-tests (push) Canceled after 0s
CI / container-build (push) Canceled after 0s
CI / dependency-audit (push) Canceled after 0s
CI / lint (push) Canceled after 0s
CI / typecheck (push) Canceled after 0s
CI / unit-tests (push) Canceled after 0s
CI / integration-tests (push) Canceled after 0s
CI / container-build (push) Canceled after 0s
CI / dependency-audit (push) Canceled after 0s
Section 10: every image API responses reference has pointed straight at TMDb's CDN since day one -- Aurora never actually downloaded or served artwork itself, despite storing dimensions/checksum/local_path columns for exactly that. Closes the gap: - is_primary was declared on the Artwork model but never actually set anywhere -- every row defaulted to False. Now the normalizer marks the one image each provider designates as *the* image for an entity (TMDb's poster_path/backdrop_path/profile_path) as primary, distinct from the dozens of alternates also ingested, including correctly marking an existing row primary (not inserting a duplicate) when that image already appears in the alternates list. - app/services/artwork/: an ArtworkStorage interface with a LocalDiskStorage implementation (checksum-named files, single-host deployment via a Docker volume shared between api and worker -- see docs/architecture.md for the S3/R2 upgrade path), and a download pipeline with content-type validation, a size cap, and path-traversal protection. - After movie/series/person ingestion succeeds, tasks.py cascades one download_artwork_task per primary artwork id via arq's ctx['redis'] (same fan-out pattern as the season cascade) -- fire-and-forget, not tracked in ingestion_jobs, since a failed image cache is a known-safe fallback to the provider URL, not a user-facing failure. - app/main.py serves MEDIA_ROOT at MEDIA_URL_PREFIX; local_path is preferred over original_url once set (already-existing fallback logic in app/services/common.py:artwork_url, now actually exercised). Also fixes a real interaction bug this surfaced: every ingestion pipeline deleted and recreated all of an entity's Artwork rows on each re-sync, which would have silently wiped local_path/checksum on every re-ingestion, forcing a full re-download every time and defeating the point of caching locally at all. app/repositories/polymorphic.py's new sync_artwork upserts by provider_asset_id (TMDb's stable per-image path) instead, preserving downloaded copies across re-sync, and replaces three near-identical delete+recreate helpers that used to live separately in movie_ingestion.py, series_ingestion.py, and person_ingestion.py. Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
cd4dd98adb
commit
fef914db36
@@ -0,0 +1,110 @@
|
||||
"""Downloads one Artwork row's original_url and stores it locally
|
||||
(section 10; the "Artwork processing" background worker from section 1's
|
||||
architecture diagram).
|
||||
|
||||
Deliberately narrow: only ever called for artwork already marked
|
||||
`is_primary` (see `app/services/ingestion/runner.py:get_primary_artwork_ids`
|
||||
and `app/workers/tasks.py`, which cascade a download job per primary image
|
||||
after each ingestion succeeds) — TMDb returns dozens of backdrop/logo
|
||||
alternates per title, and downloading every one of them automatically would
|
||||
be a lot of unnecessary storage and bandwidth for images Aurora's own
|
||||
clients are unlikely to ever show. Downloading on request for anything
|
||||
beyond the primary image is a reasonable follow-up if that's ever needed.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import uuid
|
||||
|
||||
import httpx
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.core.config import get_settings
|
||||
from app.core.logging import get_logger
|
||||
from app.models.artwork import Artwork
|
||||
from app.services.artwork.storage import ArtworkStorage
|
||||
|
||||
logger = get_logger(__name__)
|
||||
|
||||
_EXTENSION_BY_CONTENT_TYPE = {
|
||||
"image/jpeg": "jpg",
|
||||
"image/jpg": "jpg",
|
||||
"image/png": "png",
|
||||
"image/webp": "webp",
|
||||
"image/gif": "gif",
|
||||
"image/svg+xml": "svg",
|
||||
}
|
||||
|
||||
|
||||
class ArtworkDownloadError(Exception):
|
||||
"""Raised for any download-side failure (bad content-type, too large,
|
||||
network error, ...). Callers (the arq task) let this propagate so the
|
||||
job gets retried like any other ingestion failure — see
|
||||
app/services/ingestion/runner.py."""
|
||||
|
||||
|
||||
async def download_artwork(
|
||||
session: AsyncSession,
|
||||
storage: ArtworkStorage,
|
||||
artwork_id: uuid.UUID,
|
||||
*,
|
||||
client: httpx.AsyncClient | None = None,
|
||||
force: bool = False,
|
||||
) -> bool:
|
||||
"""Fetch `artwork.original_url`, store it, and set `local_path` +
|
||||
`checksum`. Returns False (no-op) if the row is gone or already has a
|
||||
local copy (unless `force=True`); True if it downloaded something.
|
||||
"""
|
||||
|
||||
artwork = await session.get(Artwork, artwork_id)
|
||||
if artwork is None:
|
||||
return False
|
||||
if artwork.local_path and not force:
|
||||
return False
|
||||
|
||||
settings = get_settings()
|
||||
owns_client = client is None
|
||||
http_client = client or httpx.AsyncClient(timeout=20.0, follow_redirects=True)
|
||||
try:
|
||||
content, content_type = await _fetch(http_client, artwork.original_url, settings.ARTWORK_DOWNLOAD_MAX_BYTES)
|
||||
finally:
|
||||
if owns_client:
|
||||
await http_client.aclose()
|
||||
|
||||
checksum = hashlib.sha256(content).hexdigest()
|
||||
extension = _EXTENSION_BY_CONTENT_TYPE.get(content_type, "jpg")
|
||||
relative_path = f"{artwork.artwork_type.value}/{checksum[:2]}/{checksum}.{extension}"
|
||||
|
||||
local_url = await storage.save(relative_path, content)
|
||||
|
||||
artwork.local_path = local_url
|
||||
artwork.checksum = checksum
|
||||
await session.flush()
|
||||
logger.info(
|
||||
"artwork_downloaded",
|
||||
artwork_id=str(artwork_id),
|
||||
checksum=checksum,
|
||||
bytes=len(content),
|
||||
)
|
||||
return True
|
||||
|
||||
|
||||
async def _fetch(client: httpx.AsyncClient, url: str, max_bytes: int) -> tuple[bytes, str]:
|
||||
try:
|
||||
async with client.stream("GET", url) as response:
|
||||
response.raise_for_status()
|
||||
content_type = response.headers.get("content-type", "").split(";")[0].strip().lower()
|
||||
if not content_type.startswith("image/"):
|
||||
raise ArtworkDownloadError(f"expected an image, got content-type={content_type!r}")
|
||||
|
||||
body = bytearray()
|
||||
async for chunk in response.aiter_bytes():
|
||||
body.extend(chunk)
|
||||
if len(body) > max_bytes:
|
||||
raise ArtworkDownloadError(
|
||||
f"artwork at {url} exceeds ARTWORK_DOWNLOAD_MAX_BYTES ({max_bytes} bytes)"
|
||||
)
|
||||
return bytes(body), content_type
|
||||
except httpx.HTTPError as exc:
|
||||
raise ArtworkDownloadError(f"failed to download {url}: {exc}") from exc
|
||||
@@ -0,0 +1,66 @@
|
||||
"""Artwork storage abstraction (section 10).
|
||||
|
||||
Downloaded artwork bytes need somewhere to live that both the worker
|
||||
(writing) and the API (serving, via app/main.py's static mount) can reach.
|
||||
`ArtworkStorage` is the seam: today there's only `LocalDiskStorage` — fine
|
||||
for a single Docker host where the api and worker containers share one
|
||||
volume (see docker-compose.yml), which is this project's actual deployment
|
||||
shape (section 1: modular monolith, not a multi-host fleet). A deployment
|
||||
that outgrows a single host would swap in an S3/R2-backed implementation
|
||||
behind this same interface without touching the download pipeline that
|
||||
calls it (app/services/artwork/download.py).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from abc import ABC, abstractmethod
|
||||
from functools import lru_cache
|
||||
from pathlib import Path
|
||||
|
||||
from app.core.config import get_settings
|
||||
|
||||
|
||||
class ArtworkStorage(ABC):
|
||||
@abstractmethod
|
||||
async def save(self, relative_path: str, content: bytes) -> str:
|
||||
"""Persist `content` at `relative_path` (already includes an
|
||||
extension, e.g. "poster/ab/ab34...jpg"). Returns the root-relative
|
||||
URL clients should use to fetch it."""
|
||||
|
||||
@abstractmethod
|
||||
async def exists(self, relative_path: str) -> bool: ...
|
||||
|
||||
|
||||
class LocalDiskStorage(ArtworkStorage):
|
||||
def __init__(self, root: str, url_prefix: str) -> None:
|
||||
self._root = Path(root)
|
||||
self._url_prefix = url_prefix.rstrip("/")
|
||||
|
||||
def _full_path(self, relative_path: str) -> Path:
|
||||
# relative_path is always built by us from a sha256 checksum, never
|
||||
# from unsanitized external input — but guard anyway against ever
|
||||
# writing outside MEDIA_ROOT.
|
||||
root = self._root.resolve()
|
||||
full = (self._root / relative_path).resolve()
|
||||
if full != root and root not in full.parents:
|
||||
raise ValueError(f"unsafe artwork path escapes MEDIA_ROOT: {relative_path!r}")
|
||||
return full
|
||||
|
||||
async def save(self, relative_path: str, content: bytes) -> str:
|
||||
full = self._full_path(relative_path)
|
||||
full.parent.mkdir(parents=True, exist_ok=True)
|
||||
# Blocking I/O, but artwork files are small (capped by
|
||||
# ARTWORK_DOWNLOAD_MAX_BYTES) and this only ever runs inside the
|
||||
# worker process, not a request handler — not worth wiring aiofiles
|
||||
# for.
|
||||
full.write_bytes(content)
|
||||
return f"{self._url_prefix}/{relative_path}"
|
||||
|
||||
async def exists(self, relative_path: str) -> bool:
|
||||
return self._full_path(relative_path).is_file()
|
||||
|
||||
|
||||
@lru_cache
|
||||
def get_artwork_storage() -> ArtworkStorage:
|
||||
settings = get_settings()
|
||||
return LocalDiskStorage(settings.MEDIA_ROOT, settings.MEDIA_URL_PREFIX)
|
||||
Reference in New Issue
Block a user