Backend upload-automation file
This commit is contained in:
@@ -4,12 +4,10 @@
|
||||
GET /api/uploads/catalog - the batches this caller has sent
|
||||
GET /api/uploads/catalog/{batch_id} - progress and result of one of them
|
||||
|
||||
This is deliberately its own router, with its own prefix and its own guard, so
|
||||
that the difference between it and everything else in the app is visible in one
|
||||
screen rather than inferred from a decorator halfway down a 400-line admin
|
||||
module. Everything else that touches catalog data is `require_admin`; this is
|
||||
`require_permission("upload_catalog")`, and that single line is the whole
|
||||
security boundary of the feature.
|
||||
This is deliberately its own router, with its own prefix, so that the difference
|
||||
between it and everything else in the app is visible in one screen rather than
|
||||
inferred from a decorator halfway down a 400-line admin module. Everything else
|
||||
that touches catalog data is `require_admin`; the POST here has no guard at all.
|
||||
|
||||
WHAT HAPPENS WHEN A FILE ARRIVES
|
||||
--------------------------------
|
||||
@@ -23,19 +21,27 @@ The response is a `batch_id`. Ingestion is far too slow to finish inside a
|
||||
request - it is thousands of rows through eleven stages - so the caller polls
|
||||
GET /api/uploads/catalog/{batch_id} until `status` leaves `queued`/`running`.
|
||||
|
||||
THIS CREDENTIAL NOW COSTS CPU, AND THAT IS THE POINT
|
||||
----------------------------------------------------
|
||||
An earlier version of this endpoint parked files in a review inbox and started
|
||||
nothing, so that a leaked key could cost only disk. That is not the product:
|
||||
an API user sends a file in order for it to be ingested, and a queue that needs
|
||||
an admin to press a button is not an API.
|
||||
AN UNAUTHENTICATED POST NOW STARTS REAL WORK
|
||||
--------------------------------------------
|
||||
Be clear-eyed about what that means. This endpoint takes no credential, and
|
||||
`UPLOAD_AUTORUN` (default true) runs the pipeline the moment a file lands. So
|
||||
"anyone who can reach this host" and "anyone who can write to the live catalog"
|
||||
are the same set of people, and an ingest is an upsert with no undo.
|
||||
|
||||
So the bound is no longer "this role cannot start work" but "all work, from
|
||||
every source, goes through one worker". `batch_worker` runs a single batch at a
|
||||
time behind a queue of `BATCH_QUEUE_MAX`; past that this endpoint answers 429.
|
||||
An uploader key can therefore occupy the ingestion worker, which is what it is
|
||||
for - it cannot multiply it, which is what matters on a one-vCPU host that is
|
||||
also serving the API and its healthcheck.
|
||||
That was chosen deliberately, over the alternative of issuing the sender an
|
||||
`uploader` API key and auto-running only credentialed requests. The requirement
|
||||
was uploads that run without manual intervention, and a review queue that needs
|
||||
an admin to press a button is not that.
|
||||
|
||||
What still bounds it is throughput, not identity: the per-request ceilings in
|
||||
`_limits()`, and `batch_worker` running a single batch at a time behind a queue
|
||||
of `BATCH_QUEUE_MAX`, past which this endpoint answers 429. A sender can occupy
|
||||
the ingestion worker - that is what it is for - but cannot multiply it, which is
|
||||
what matters on a one-vCPU host also serving the API and its healthcheck.
|
||||
|
||||
Setting `UPLOAD_AUTORUN=false` restores the review inbox, where files wait for
|
||||
an admin and the cost of an unwanted drop is disk rather than products. Both
|
||||
paths are live and both are tested; see `stage_pending` in batch_common.
|
||||
|
||||
WHAT THIS ENDPOINT STILL CANNOT DO
|
||||
----------------------------------
|
||||
@@ -62,6 +68,9 @@ from app.infrastructure.settings import (
|
||||
BATCH_MAX_TOTAL_ROWS,
|
||||
INBOX_MAX_PENDING_BYTES,
|
||||
INBOX_MAX_PENDING_FILES,
|
||||
UPLOAD_AUTORUN,
|
||||
UPLOAD_AUTORUN_FETCH_IMAGES,
|
||||
UPLOAD_AUTORUN_USE_LLM,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -145,29 +154,35 @@ async def ingest_catalog_files(
|
||||
sender: Optional[str] = Form(None),
|
||||
principal: Optional[Principal] = Depends(get_optional_principal),
|
||||
) -> CatalogUploadOut:
|
||||
"""Accept spreadsheets into the review inbox. No credential required.
|
||||
"""Accept spreadsheets and start the pipeline over them. No credential.
|
||||
|
||||
Returns 202 and a `batch_id` to poll. A drop where some files parse and some
|
||||
do not is a partial success, not a failure: the good ones are stored and the
|
||||
bad ones come back in `files` as `status: "failed"` with the reason, so the
|
||||
caller knows exactly which sheet to fix and resend.
|
||||
do not is a partial success, not a failure: the good ones are accepted and
|
||||
the bad ones come back in `files` as `status: "failed"` with the reason, so
|
||||
the caller knows exactly which sheet to fix and resend.
|
||||
|
||||
NOTHING SENT HERE RUNS ON ARRIVAL.
|
||||
The files are staged and the batch is left `pending`. An admin sees it in the
|
||||
review inbox, ticks the sheets they want and presses Start; only then does
|
||||
anything reach the pipeline or the catalog. That gate is what makes an
|
||||
endpoint anybody can post to acceptable: the cost of an unwanted drop is
|
||||
disk until someone declines it, not products in the live catalog.
|
||||
WHAT HAPPENS ON ARRIVAL depends on one setting, `UPLOAD_AUTORUN`:
|
||||
|
||||
`use_llm` and `fetch_images` are NOT accepted here, though they used to be.
|
||||
They decide how a run behaves, and the person who decides that is now the
|
||||
admin pressing Start - not the sender. Leaving them on this endpoint would
|
||||
let an anonymous caller commit the host to Playwright image search.
|
||||
true (default) the batch is queued and the 11 stages run immediately. The
|
||||
id returned IS the run id - poll it and watch `stages[]`.
|
||||
false the batch is left `pending` in the admin review inbox and
|
||||
nothing runs until someone presses Start. The id returned
|
||||
is a DROP id; the run gets a different one, reachable
|
||||
through the file's `released_to`.
|
||||
|
||||
Clients should not care which is configured: both return 202 and an id that
|
||||
`GET /api/uploads/catalog/{id}` understands. Only the number of hops differs.
|
||||
|
||||
`use_llm` and `fetch_images` are still NOT accepted from the request, and
|
||||
that has not changed with autorun - if anything it matters more. The caller
|
||||
is anonymous, and letting an anonymous caller switch on the expensive
|
||||
outbound stages is the one thing this endpoint must not allow. They come
|
||||
from `UPLOAD_AUTORUN_FETCH_IMAGES` / `UPLOAD_AUTORUN_USE_LLM` instead.
|
||||
|
||||
`sender` is a free-text label, not identity - it is whatever the caller
|
||||
typed. It exists because the inbox groups drops by who sent them, and three
|
||||
colleagues all showing as "anonymous" is an inbox nobody can triage. A real
|
||||
credential, if one is presented, wins over it.
|
||||
typed. It exists so a run can be attributed to a person, and three
|
||||
colleagues all showing as "anonymous" is a batch list nobody can triage. A
|
||||
real credential, if one is presented, wins over it.
|
||||
"""
|
||||
limits = _limits()
|
||||
read = await batch_common.read_uploads(files, limits)
|
||||
@@ -180,10 +195,15 @@ async def ingest_catalog_files(
|
||||
detail=f"None of the uploaded files could be ingested. {detail}",
|
||||
)
|
||||
|
||||
_inbox_capacity_or_429(
|
||||
incoming_files=len(valid),
|
||||
incoming_bytes=sum(len(contents) for _n, contents, _r in valid),
|
||||
)
|
||||
# Only meaningful on the review-inbox path. Under autorun nothing ever
|
||||
# awaits review, so the count it guards is permanently zero and the check
|
||||
# could never fire - and a guard that cannot guard anything reads, to the
|
||||
# next person, like protection that is actually there.
|
||||
if not UPLOAD_AUTORUN:
|
||||
_inbox_capacity_or_429(
|
||||
incoming_files=len(valid),
|
||||
incoming_bytes=sum(len(contents) for _n, contents, _r in valid),
|
||||
)
|
||||
|
||||
# A presented credential still names the sender - `get_optional_principal`
|
||||
# returns None only when NO credential was sent, and still raises on one
|
||||
@@ -194,30 +214,68 @@ async def ingest_catalog_files(
|
||||
else ((sender or "").strip()[:60] or "anonymous")
|
||||
)
|
||||
|
||||
manifest = batch_common.stage_pending(
|
||||
valid,
|
||||
invalid,
|
||||
submitted_by=submitted_by,
|
||||
)
|
||||
|
||||
logger.info(
|
||||
"Catalog drop %s received for review: %d file(s), %d rejected, from %s%s",
|
||||
manifest.batch_id, len(valid), len(invalid), submitted_by,
|
||||
"" if principal else " (no credential)",
|
||||
)
|
||||
if UPLOAD_AUTORUN:
|
||||
manifest, started = batch_common.stage_and_queue(
|
||||
valid,
|
||||
invalid,
|
||||
use_llm=UPLOAD_AUTORUN_USE_LLM,
|
||||
fetch_images=UPLOAD_AUTORUN_FETCH_IMAGES,
|
||||
submitted_by=submitted_by,
|
||||
)
|
||||
if not started:
|
||||
# Staged and durable, but not running, and this caller has no Resume
|
||||
# button - that lives on the admin router. So the honest instruction
|
||||
# is to send it again shortly. The id is named so an admin can find
|
||||
# and resume THIS batch instead, if the caller reports it.
|
||||
raise HTTPException(
|
||||
status_code=429,
|
||||
detail=(
|
||||
f"Too many batches are already queued. Batch "
|
||||
f"{manifest.batch_id} has been saved but not started; retry "
|
||||
f"this upload shortly."
|
||||
),
|
||||
)
|
||||
logger.info(
|
||||
"Catalog batch %s queued: %d file(s), %d rejected, from %s%s",
|
||||
manifest.batch_id, len(valid), len(invalid), submitted_by,
|
||||
"" if principal else " (no credential)",
|
||||
)
|
||||
else:
|
||||
manifest = batch_common.stage_pending(
|
||||
valid,
|
||||
invalid,
|
||||
submitted_by=submitted_by,
|
||||
)
|
||||
logger.info(
|
||||
"Catalog drop %s received for review: %d file(s), %d rejected, from %s%s",
|
||||
manifest.batch_id, len(valid), len(invalid), submitted_by,
|
||||
"" if principal else " (no credential)",
|
||||
)
|
||||
|
||||
body = batch_common.to_out(manifest).model_dump()
|
||||
message = (
|
||||
f"{len(valid)} file(s) received and waiting for review. Nothing runs "
|
||||
f"until an admin starts them. "
|
||||
f"Poll GET /api/uploads/catalog/{manifest.batch_id} for status."
|
||||
)
|
||||
if invalid:
|
||||
if UPLOAD_AUTORUN:
|
||||
message = (
|
||||
f"{len(valid)} file(s) received and waiting for review. "
|
||||
f"{len(invalid)} could not be read - see 'files' for the reason on "
|
||||
f"each, and resend those."
|
||||
f"{len(valid)} file(s) accepted and queued for ingestion. "
|
||||
f"Poll GET /api/uploads/catalog/{manifest.batch_id} for progress."
|
||||
)
|
||||
if invalid:
|
||||
message = (
|
||||
f"{len(valid)} file(s) accepted and queued for ingestion. "
|
||||
f"{len(invalid)} could not be read - see 'files' for the reason "
|
||||
f"on each, and resend those."
|
||||
)
|
||||
else:
|
||||
message = (
|
||||
f"{len(valid)} file(s) received and waiting for review. Nothing runs "
|
||||
f"until an admin starts them. "
|
||||
f"Poll GET /api/uploads/catalog/{manifest.batch_id} for status."
|
||||
)
|
||||
if invalid:
|
||||
message = (
|
||||
f"{len(valid)} file(s) received and waiting for review. "
|
||||
f"{len(invalid)} could not be read - see 'files' for the reason on "
|
||||
f"each, and resend those."
|
||||
)
|
||||
return CatalogUploadOut(**body, message=message)
|
||||
|
||||
|
||||
|
||||
@@ -163,10 +163,42 @@ BATCH_QUEUE_MAX = int(os.getenv("BATCH_QUEUE_MAX", "4"))
|
||||
# resource it has.
|
||||
BATCH_RETENTION_DAYS = int(os.getenv("BATCH_RETENTION_DAYS", "7"))
|
||||
|
||||
# --- Unattended ingestion --------------------------------------------------
|
||||
# Whether POST /api/uploads/catalog runs the pipeline on arrival, or parks the
|
||||
# files in the admin review inbox for someone to start by hand.
|
||||
#
|
||||
# READ THIS BEFORE CHANGING IT. That endpoint takes NO credential - it was
|
||||
# opened deliberately so colleagues could send spreadsheets without one being
|
||||
# issued to them. With autorun on, "anyone who can reach this host" and "anyone
|
||||
# who can write to the live catalogue" become the same set of people, and an
|
||||
# ingest is an upsert with no undo. That trade was made knowingly: the ask was
|
||||
# for uploads to run without manual intervention, and a review queue that needs
|
||||
# an admin to press a button is not that.
|
||||
#
|
||||
# What still bounds it: the per-request ceilings above (20 files / 50MB / 20k
|
||||
# rows), and BATCH_QUEUE_MAX behind a single worker thread - so a sender can
|
||||
# occupy the ingestion worker but cannot multiply it. Those cap throughput, not
|
||||
# who. If that stops being an acceptable trade, set this to false and the review
|
||||
# inbox comes back with no code change; everything it needs is still here.
|
||||
UPLOAD_AUTORUN = _bool("UPLOAD_AUTORUN", "true")
|
||||
|
||||
# How an auto-started run behaves. Not accepted from the request: the sender is
|
||||
# anonymous, and letting an anonymous caller turn on the expensive stages is the
|
||||
# one thing the open endpoint must not allow.
|
||||
#
|
||||
# Images ON, because a product landing without one is the failure this endpoint
|
||||
# exists to avoid - stage 6 is the slowest stage and reaches the network, but
|
||||
# only one batch runs at a time so nothing else is competing with it.
|
||||
#
|
||||
# LLM OFF, because `use_llm` gates only description generation in
|
||||
# stage_2_row_intake, and production runs USE_OLLAMA=false: turning it on there
|
||||
# buys nothing and costs a connection timeout per row.
|
||||
UPLOAD_AUTORUN_FETCH_IMAGES = _bool("UPLOAD_AUTORUN_FETCH_IMAGES", "true")
|
||||
UPLOAD_AUTORUN_USE_LLM = _bool("UPLOAD_AUTORUN_USE_LLM", "false")
|
||||
|
||||
# --- Review inbox ----------------------------------------------------------
|
||||
# POST /api/uploads/catalog accepts files with NO credential, so that colleagues
|
||||
# can send spreadsheets without one being issued to them. Nothing it accepts is
|
||||
# queued - files wait in the admin review inbox - which removes BATCH_QUEUE_MAX
|
||||
# The bound that applies only when UPLOAD_AUTORUN is false. Files then wait in
|
||||
# the admin review inbox rather than being queued, which removes BATCH_QUEUE_MAX
|
||||
# as the bound on that endpoint and leaves the volume as the only thing an
|
||||
# anonymous sender can exhaust. These are that bound; past either, the endpoint
|
||||
# answers 429 and stages nothing.
|
||||
|
||||
Reference in New Issue
Block a user