From 669034896aa3b37312dd0bc8ecfa803237629e89 Mon Sep 17 00:00:00 2001 From: NixReaper Date: Fri, 4 Sep 2026 19:28:31 -0500 Subject: [PATCH] Fix job_id mismatch between ingestion enqueue and status endpoints POST /api/v1/admin/ingest/movie returned arq's own internal queue job id, but GET /api/v1/admin/ingest/jobs/{job_id} looked up our IngestionJob DB row by its own UUID -- a completely different id that didn't even exist yet at enqueue time (the row was only created once the worker picked up the task). Every status poll 404'd. Fix: create the IngestionJob row synchronously in the enqueue endpoint (reusing the existing idempotency-key logic, so repeat enqueues for the same tmdb_id reuse the same row rather than creating duplicates), pass its real id into the arq task, and have the worker update that row instead of creating a new one. Found by actually running the milestone end-to-end: enqueue a real TMDb ingestion, then poll its status. Co-Authored-By: Claude Sonnet 5 --- app/api/v1/admin.py | 19 +++++++++++++------ app/services/ingestion/runner.py | 23 ++++++++++++++++++----- app/workers/tasks.py | 14 ++++++++++---- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/app/api/v1/admin.py b/app/api/v1/admin.py index d285c38..e691cce 100644 --- a/app/api/v1/admin.py +++ b/app/api/v1/admin.py @@ -17,6 +17,7 @@ from app.core.database import get_db from app.core.queue import get_queue from app.core.security import require_admin_api_key from app.repositories.provider_repository import get_ingestion_job +from app.services.ingestion.runner import get_or_create_ingestion_job router = APIRouter( prefix="/admin", @@ -54,24 +55,30 @@ class IngestionJobOut(BaseModel): async def enqueue_movie_ingestion( body: IngestMovieRequest, queue: ArqRedis = Depends(get_queue) ) -> IngestionEnqueuedOut: - job = await queue.enqueue_job("ingest_movie_task", body.tmdb_id) - if job is None: + # Created synchronously (and idempotently — same tmdb_id reuses the same + # row) so the response's job_id is immediately valid against + # GET /ingest/jobs/{job_id}. arq's own job id is a separate, unrelated + # identifier used only for queue bookkeeping. + job_id = await get_or_create_ingestion_job("movie_sync", "movie", body.tmdb_id) + arq_job = await queue.enqueue_job("ingest_movie_task", str(job_id), body.tmdb_id) + if arq_job is None: raise HTTPException( status.HTTP_503_SERVICE_UNAVAILABLE, "Could not enqueue ingestion job (queue busy)." ) - return IngestionEnqueuedOut(job_id=job.job_id, arq_job_id=job.job_id) + return IngestionEnqueuedOut(job_id=str(job_id), arq_job_id=arq_job.job_id) @router.post("/ingest/person", response_model=IngestionEnqueuedOut) async def enqueue_person_ingestion( body: IngestPersonRequest, queue: ArqRedis = Depends(get_queue) ) -> IngestionEnqueuedOut: - job = await queue.enqueue_job("ingest_person_task", body.tmdb_id) - if job is None: + job_id = await get_or_create_ingestion_job("person_sync", "person", body.tmdb_id) + arq_job = await queue.enqueue_job("ingest_person_task", str(job_id), body.tmdb_id) + if arq_job is None: raise HTTPException( status.HTTP_503_SERVICE_UNAVAILABLE, "Could not enqueue ingestion job (queue busy)." ) - return IngestionEnqueuedOut(job_id=job.job_id, arq_job_id=job.job_id) + return IngestionEnqueuedOut(job_id=str(job_id), arq_job_id=arq_job.job_id) @router.get("/ingest/jobs/{job_id}", response_model=IngestionJobOut) diff --git a/app/services/ingestion/runner.py b/app/services/ingestion/runner.py index 5414be3..dedb033 100644 --- a/app/services/ingestion/runner.py +++ b/app/services/ingestion/runner.py @@ -38,7 +38,18 @@ async def _require_job(session: AsyncSession, job_id: uuid.UUID) -> IngestionJob return job -async def _create_job(job_type: str, entity_type: str, external_id: str) -> uuid.UUID: +async def get_or_create_ingestion_job(job_type: str, entity_type: str, external_id: str) -> uuid.UUID: + """Create the DB-tracked `IngestionJob` row (or return the existing one + for this provider+entity+external_id, keyed by `idempotency_key`) + without running the pipeline. + + Called synchronously from the admin endpoint, *before* enqueuing to arq, + specifically so the HTTP response can return a `job_id` that + `GET /admin/ingest/jobs/{job_id}` can immediately resolve — arq's own + queue-internal job id is a different, unrelated identifier and was + never valid here. + """ + idempotency_key = f"tmdb:{entity_type}:{external_id}" async with session_scope() as session: provider_row = await get_or_create_provider(session, "tmdb", "The Movie Database (TMDb)") @@ -53,8 +64,9 @@ async def _create_job(job_type: str, entity_type: str, external_id: str) -> uuid return job.id -async def run_movie_ingestion(external_id: str) -> uuid.UUID: - job_id = await _create_job("movie_sync", "movie", external_id) +async def run_movie_ingestion(job_id: uuid.UUID, external_id: str) -> uuid.UUID: + """Run the pipeline for an `IngestionJob` row that already exists (see + `get_or_create_ingestion_job`) — this never creates the row itself.""" async with session_scope() as session: job = await _require_job(session, job_id) @@ -78,8 +90,9 @@ async def run_movie_ingestion(external_id: str) -> uuid.UUID: await provider.aclose() -async def run_person_ingestion(external_id: str) -> uuid.UUID: - job_id = await _create_job("person_sync", "person", external_id) +async def run_person_ingestion(job_id: uuid.UUID, external_id: str) -> uuid.UUID: + """Run the pipeline for an `IngestionJob` row that already exists (see + `get_or_create_ingestion_job`) — this never creates the row itself.""" async with session_scope() as session: job = await _require_job(session, job_id) diff --git a/app/workers/tasks.py b/app/workers/tasks.py index 7e03d07..2910cf4 100644 --- a/app/workers/tasks.py +++ b/app/workers/tasks.py @@ -1,9 +1,15 @@ """arq task functions. Thin wrappers around the ingestion runner so retry/ concurrency/observability behavior lives in arq's worker loop, not here. + +Each task receives the `IngestionJob` row's id (created synchronously by the +enqueuing endpoint via `get_or_create_ingestion_job`, *not* generated here) +so status polling has a real, immediately-valid id to look up from the +moment the HTTP response comes back. """ from __future__ import annotations +import uuid from typing import Any from app.core.logging import get_logger @@ -12,11 +18,11 @@ from app.services.ingestion.runner import run_movie_ingestion, run_person_ingest logger = get_logger(__name__) -async def ingest_movie_task(ctx: dict[str, Any], external_id: str) -> str: - movie_id = await run_movie_ingestion(external_id) +async def ingest_movie_task(ctx: dict[str, Any], job_id: str, external_id: str) -> str: + movie_id = await run_movie_ingestion(uuid.UUID(job_id), external_id) return str(movie_id) -async def ingest_person_task(ctx: dict[str, Any], external_id: str) -> str: - person_id = await run_person_ingestion(external_id) +async def ingest_person_task(ctx: dict[str, Any], job_id: str, external_id: str) -> str: + person_id = await run_person_ingestion(uuid.UUID(job_id), external_id) return str(person_id)