Backend fix checkbox
This commit is contained in:
@@ -65,9 +65,10 @@ async def read_uploads(
|
||||
) -> List[Tuple[str, bytes]]:
|
||||
"""Read every upload into memory, enforcing the count and size ceilings.
|
||||
|
||||
Read here rather than in the worker for the reason `store_catalog.py` gives:
|
||||
`UploadFile` is backed by a temporary file tied to the request, and it is
|
||||
gone before a background thread would reach it.
|
||||
Read here rather than in the worker because `UploadFile` is backed by a
|
||||
temporary file tied to the request: FastAPI closes it when the response is
|
||||
returned, so a background thread reaching for it later finds nothing. The
|
||||
bytes have to be taken while the request is still alive.
|
||||
"""
|
||||
if not files:
|
||||
raise HTTPException(status_code=400, detail="No files were uploaded.")
|
||||
@@ -114,9 +115,10 @@ async def read_uploads(
|
||||
def parse_all(read: List[Tuple[str, bytes]], limits: UploadLimits):
|
||||
"""Split the uploads into (valid, invalid) by trying to parse each one.
|
||||
|
||||
Failing here is what stops a batch transitioning straight to "failed" a
|
||||
second after it started - the same reasoning as `store_catalog.py`, applied
|
||||
per file so one bad sheet does not condemn the others.
|
||||
Parsing up front is what stops a batch transitioning straight to "failed" a
|
||||
second after it started: an unreadable sheet is rejected in the response the
|
||||
caller is still waiting on, not in a manifest they have to go and poll for.
|
||||
Applied per file, so one bad sheet does not condemn the others.
|
||||
"""
|
||||
valid: List[Tuple[str, bytes, int]] = []
|
||||
invalid: List[Tuple[str, str]] = []
|
||||
|
||||
@@ -10,20 +10,25 @@
|
||||
POST /api/admin/catalog-batch/from-inbox - run selected files
|
||||
POST /api/admin/catalog-batch/inbox/dismiss - discard selected files
|
||||
|
||||
This is the multi-file sibling of `store_catalog.py`, and it deliberately does
|
||||
not replace it: the single-file endpoints are untouched and still work. What is
|
||||
different is the unit of work. Five files are one batch with one id, so the
|
||||
question a colleague actually asks - "did the drop land?" - has one answer
|
||||
rather than five.
|
||||
These began as the multi-file sibling of a single-file admin uploader
|
||||
(`store_catalog.py` and its Store Catalog Ingestion tab). THAT UPLOADER AND ITS
|
||||
ROUTES HAVE SINCE BEEN DELETED - do not go looking for them - and this router is
|
||||
now the only admin way in. What made it the survivor is the unit of work: five
|
||||
files are one batch with one id, so the question a colleague actually asks -
|
||||
"did the drop land?" - has one answer rather than five.
|
||||
|
||||
WHY THE NETWORK STAGES DEFAULT OFF HERE
|
||||
---------------------------------------
|
||||
The single-file UI sends `use_llm=true, fetch_images=true`. At one file that is
|
||||
a considered trade. At twenty it is thousands of outbound requests and, for the
|
||||
image stage, a Playwright subprocess that can burn three minutes on its own -
|
||||
on a single-vCPU container that is also serving the API. So a batch opts IN to
|
||||
those stages; it does not opt out. `USE_OLLAMA` is false in production anyway,
|
||||
which makes `use_llm` a no-op there and the honest default obvious.
|
||||
Turning both on for a single file is a considered trade. At twenty files it is
|
||||
thousands of outbound requests and, for the image stage, a Playwright subprocess
|
||||
that can burn three minutes on its own - on a single-vCPU container that is also
|
||||
serving the API. So a batch opts IN to those stages; it does not opt out.
|
||||
`USE_OLLAMA` is false in production anyway, which makes `use_llm` a no-op there
|
||||
and the honest default obvious.
|
||||
|
||||
Note that these defaults bind THIS router only. The open upload endpoint runs
|
||||
itself and takes its two flags from `UPLOAD_AUTORUN_FETCH_IMAGES` and
|
||||
`UPLOAD_AUTORUN_USE_LLM`, both true - see the section below.
|
||||
|
||||
THE OTHER WAY INTO THE SAME PIPELINE
|
||||
------------------------------------
|
||||
@@ -54,9 +59,10 @@ from app.infrastructure.settings import (
|
||||
|
||||
router = APIRouter(prefix="/admin/catalog-batch", tags=["admin", "catalog"])
|
||||
|
||||
# Per-file ceilings match store_catalog.py exactly. A file that is too big for
|
||||
# the single-file endpoint is not somehow acceptable because it arrived with
|
||||
# four friends.
|
||||
# Per-file ceilings. They bound ONE spreadsheet, and they hold whether it
|
||||
# arrived alone or with nineteen friends: a file too big to parse safely does
|
||||
# not become acceptable by being part of a batch. The batch-wide ceilings
|
||||
# (BATCH_MAX_*, imported above) then bound the set on top of these.
|
||||
MAX_UPLOAD_BYTES = 10 * 1024 * 1024
|
||||
MAX_UPLOAD_ROWS = 2000
|
||||
PREVIEW_ROWS = 10
|
||||
|
||||
@@ -15,10 +15,11 @@ file. What is new is everything *around* a file:
|
||||
|
||||
WHY THE FILES GO TO DISK
|
||||
------------------------
|
||||
The single-file path reads the upload into memory and hands the bytes to a
|
||||
daemon thread (store_catalog.py). That is fine for one file and one operator
|
||||
watching it: if the process dies, they re-upload. A five-file batch is a
|
||||
different proposition - the colleague who sent them is not sitting there, and
|
||||
The obvious cheap design - read the upload into memory and hand the bytes to a
|
||||
daemon thread - is fine for one file and one operator watching it: if the
|
||||
process dies, they re-upload. (An earlier single-file admin route did exactly
|
||||
that; it has since been removed.) A five-file batch is a different proposition
|
||||
- the colleague who sent them is not sitting there, and
|
||||
silently losing the drop is worse than any amount of extra code. So the bytes
|
||||
are staged under BATCH_UPLOAD_DIR, which is on the container's declared volume,
|
||||
and a manifest records what state each file reached.
|
||||
|
||||
@@ -148,8 +148,12 @@ MODEL_ARTIFACTS_DIR = _dir(
|
||||
# endpoints - whatever Traefik defaults to is, and it is not ours to rely on.
|
||||
BATCH_UPLOAD_DIR = _dir("BATCH_UPLOAD_DIR", DATA_DIR / "batch_uploads")
|
||||
|
||||
# Per-file limits stay at the single-upload values (10MB / 2000 rows, see
|
||||
# app/api/routers/store_catalog.py); these bound the BATCH on top of that.
|
||||
# Per-file limits stay at the single-upload values, 10MB / 2000 rows. They are
|
||||
# NOT settings and are not read from here: each router declares its own
|
||||
# MAX_UPLOAD_BYTES / MAX_UPLOAD_ROWS pair with the same two numbers - see
|
||||
# routers/batch_catalog.py, routers/uploads.py and routers/user_products.py.
|
||||
# Change one and you have changed one. The ceilings below bound the BATCH on
|
||||
# top of whichever per-file pair applied.
|
||||
BATCH_MAX_FILES = int(os.getenv("BATCH_MAX_FILES", "20"))
|
||||
BATCH_MAX_TOTAL_BYTES = int(os.getenv("BATCH_MAX_TOTAL_BYTES", str(50 * 1024 * 1024)))
|
||||
BATCH_MAX_TOTAL_ROWS = int(os.getenv("BATCH_MAX_TOTAL_ROWS", "20000"))
|
||||
|
||||
Reference in New Issue
Block a user