Fix job_id mismatch between ingestion enqueue and status endpoints
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
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 <[email protected]>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
dd6ecce78d
commit
669034896a
+13
-6
@@ -17,6 +17,7 @@ from app.core.database import get_db
|
|||||||
from app.core.queue import get_queue
|
from app.core.queue import get_queue
|
||||||
from app.core.security import require_admin_api_key
|
from app.core.security import require_admin_api_key
|
||||||
from app.repositories.provider_repository import get_ingestion_job
|
from app.repositories.provider_repository import get_ingestion_job
|
||||||
|
from app.services.ingestion.runner import get_or_create_ingestion_job
|
||||||
|
|
||||||
router = APIRouter(
|
router = APIRouter(
|
||||||
prefix="/admin",
|
prefix="/admin",
|
||||||
@@ -54,24 +55,30 @@ class IngestionJobOut(BaseModel):
|
|||||||
async def enqueue_movie_ingestion(
|
async def enqueue_movie_ingestion(
|
||||||
body: IngestMovieRequest, queue: ArqRedis = Depends(get_queue)
|
body: IngestMovieRequest, queue: ArqRedis = Depends(get_queue)
|
||||||
) -> IngestionEnqueuedOut:
|
) -> IngestionEnqueuedOut:
|
||||||
job = await queue.enqueue_job("ingest_movie_task", body.tmdb_id)
|
# Created synchronously (and idempotently — same tmdb_id reuses the same
|
||||||
if job is None:
|
# 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(
|
raise HTTPException(
|
||||||
status.HTTP_503_SERVICE_UNAVAILABLE, "Could not enqueue ingestion job (queue busy)."
|
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)
|
@router.post("/ingest/person", response_model=IngestionEnqueuedOut)
|
||||||
async def enqueue_person_ingestion(
|
async def enqueue_person_ingestion(
|
||||||
body: IngestPersonRequest, queue: ArqRedis = Depends(get_queue)
|
body: IngestPersonRequest, queue: ArqRedis = Depends(get_queue)
|
||||||
) -> IngestionEnqueuedOut:
|
) -> IngestionEnqueuedOut:
|
||||||
job = await queue.enqueue_job("ingest_person_task", body.tmdb_id)
|
job_id = await get_or_create_ingestion_job("person_sync", "person", body.tmdb_id)
|
||||||
if job is None:
|
arq_job = await queue.enqueue_job("ingest_person_task", str(job_id), body.tmdb_id)
|
||||||
|
if arq_job is None:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status.HTTP_503_SERVICE_UNAVAILABLE, "Could not enqueue ingestion job (queue busy)."
|
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)
|
@router.get("/ingest/jobs/{job_id}", response_model=IngestionJobOut)
|
||||||
|
|||||||
@@ -38,7 +38,18 @@ async def _require_job(session: AsyncSession, job_id: uuid.UUID) -> IngestionJob
|
|||||||
return job
|
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}"
|
idempotency_key = f"tmdb:{entity_type}:{external_id}"
|
||||||
async with session_scope() as session:
|
async with session_scope() as session:
|
||||||
provider_row = await get_or_create_provider(session, "tmdb", "The Movie Database (TMDb)")
|
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
|
return job.id
|
||||||
|
|
||||||
|
|
||||||
async def run_movie_ingestion(external_id: str) -> uuid.UUID:
|
async def run_movie_ingestion(job_id: uuid.UUID, external_id: str) -> uuid.UUID:
|
||||||
job_id = await _create_job("movie_sync", "movie", external_id)
|
"""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:
|
async with session_scope() as session:
|
||||||
job = await _require_job(session, job_id)
|
job = await _require_job(session, job_id)
|
||||||
@@ -78,8 +90,9 @@ async def run_movie_ingestion(external_id: str) -> uuid.UUID:
|
|||||||
await provider.aclose()
|
await provider.aclose()
|
||||||
|
|
||||||
|
|
||||||
async def run_person_ingestion(external_id: str) -> uuid.UUID:
|
async def run_person_ingestion(job_id: uuid.UUID, external_id: str) -> uuid.UUID:
|
||||||
job_id = await _create_job("person_sync", "person", external_id)
|
"""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:
|
async with session_scope() as session:
|
||||||
job = await _require_job(session, job_id)
|
job = await _require_job(session, job_id)
|
||||||
|
|||||||
+10
-4
@@ -1,9 +1,15 @@
|
|||||||
"""arq task functions. Thin wrappers around the ingestion runner so retry/
|
"""arq task functions. Thin wrappers around the ingestion runner so retry/
|
||||||
concurrency/observability behavior lives in arq's worker loop, not here.
|
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
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import uuid
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from app.core.logging import get_logger
|
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__)
|
logger = get_logger(__name__)
|
||||||
|
|
||||||
|
|
||||||
async def ingest_movie_task(ctx: dict[str, Any], external_id: str) -> str:
|
async def ingest_movie_task(ctx: dict[str, Any], job_id: str, external_id: str) -> str:
|
||||||
movie_id = await run_movie_ingestion(external_id)
|
movie_id = await run_movie_ingestion(uuid.UUID(job_id), external_id)
|
||||||
return str(movie_id)
|
return str(movie_id)
|
||||||
|
|
||||||
|
|
||||||
async def ingest_person_task(ctx: dict[str, Any], external_id: str) -> str:
|
async def ingest_person_task(ctx: dict[str, Any], job_id: str, external_id: str) -> str:
|
||||||
person_id = await run_person_ingestion(external_id)
|
person_id = await run_person_ingestion(uuid.UUID(job_id), external_id)
|
||||||
return str(person_id)
|
return str(person_id)
|
||||||
|
|||||||
Reference in New Issue
Block a user