fix(collector,trainer): migrate an existing collector DB on open; trainer handlers answer 500 JSON
The reviewer host's collector.sqlite was created by an earlier build, before the `kind` column. CREATE TABLE IF NOT EXISTS shapes only a new database, so every query naming the column failed: the collector's /health (container unhealthy), every booth ingest, and the trainer's readiness — whose stdlib server printed the traceback and dropped the socket, which the collector could only render as "trainer not reachable: fetch failed". Nine days like that. - CollectorDb.#migrate(): PRAGMA table_info against the list of columns added since the first deploy; ALTER TABLE ADD COLUMN for each missing one (all nullable or defaulted). Append to that list whenever a column joins the CREATE. Test replays the original schema: health, ingest, stats, a legacy row reads back with the defaults. - Trainer Handler._guarded(): any unexpected exception → 500 JSON naming it, never a dropped connection; /health keeps answering. Test drives readiness against an old-schema DB. - The collector's training status proxy includes the trainer's error text. Wiki: the incident and the schema rule (vision-review-outbox), what the message means (bodytype-classifier-training), log. Deploy: the new collector migrates on start; nothing manual. Claude-Session: https://claude.ai/code/session_01FWncR69HgGPuei1dLrW3cU
This commit is contained in:
@@ -151,3 +151,46 @@ describe("config", () => {
|
||||
expect(() => parseBoothTokens("nocolon")).toThrow(/bad pair/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("schema migration", () => {
|
||||
it("opens a database created before the `kind` column and adds the missing columns, so ingest and stats work", async () => {
|
||||
// art-docker-station, 2026-09-16: the volume's DB predated `kind`; CREATE TABLE IF NOT
|
||||
// EXISTS left it alone, and /health, every ingest and the trainer's readiness failed
|
||||
// with "no such column: kind". Replay: a file with the ORIGINAL column set.
|
||||
const { default: Database } = await import("better-sqlite3");
|
||||
const file = path.join(dir, "old.sqlite");
|
||||
const old = new Database(file);
|
||||
old.exec(`CREATE TABLE items (
|
||||
id TEXT PRIMARY KEY, booth TEXT NOT NULL, order_ref TEXT NOT NULL, at TEXT NOT NULL,
|
||||
service TEXT NOT NULL, vision_class TEXT NOT NULL, vision_confidence REAL NOT NULL,
|
||||
image_width INTEGER NOT NULL, image_height INTEGER NOT NULL, plate_blurred INTEGER NOT NULL,
|
||||
image_path TEXT NOT NULL, received_at TEXT NOT NULL, review_label TEXT, reviewed_at TEXT, reviewer TEXT)`);
|
||||
old.prepare(
|
||||
"INSERT INTO items (id, booth, order_ref, at, service, vision_class, vision_confidence, image_width, image_height, plate_blurred, image_path, received_at) VALUES (?,?,?,?,?,?,?,?,?,?,?,?)",
|
||||
).run("legacy-1", "booth-7", "o-0", "2026-09-01T00:00:00.000Z", "Standard", "suv", 0.8, 100, 100, 1, "crops/legacy-1.jpg", "2026-09-01T00:00:00.000Z");
|
||||
old.close();
|
||||
|
||||
const legacy = await buildCollector({ host: "127.0.0.1", port: 0, dataDir: dir, boothTokens: TOKENS, reviewer: REVIEWER, trainerUrl: null }, { dbFile: file });
|
||||
await legacy.ready();
|
||||
try {
|
||||
const health = await legacy.inject({ method: "GET", url: "/health" });
|
||||
expect(health.statusCode).toBe(200);
|
||||
expect(health.json()).toMatchObject({ ok: true, booths: 1, pending: 1 });
|
||||
|
||||
const { body, type } = multipart({ meta: JSON.stringify(meta({ item: "item-new" })) }, JPEG);
|
||||
const r = await legacy.inject({ method: "POST", url: "/ingest", headers: { authorization: `Bearer ${TOKENS.get("booth-7")!}`, "content-type": type }, payload: body });
|
||||
expect(r.statusCode).toBe(201);
|
||||
|
||||
const stats = await legacy.inject({ method: "GET", url: "/api/stats", headers: { authorization: basic } });
|
||||
expect(stats.statusCode).toBe(200);
|
||||
|
||||
// The legacy row reads back with the defaults the new columns carry.
|
||||
const cols = new Database(file, { readonly: true }).prepare("PRAGMA table_info(items)").all() as { name: string }[];
|
||||
expect(cols.map((c) => c.name)).toEqual(expect.arrayContaining(["kind", "operator_ref", "operator_classes", "vision_category_id", "downgraded"]));
|
||||
const legacyRow = new Database(file, { readonly: true }).prepare("SELECT kind, operator_classes, downgraded FROM items WHERE id = 'legacy-1'").get() as Record<string, unknown>;
|
||||
expect(legacyRow).toEqual({ kind: "wash", operator_classes: "[]", downgraded: 0 });
|
||||
} finally {
|
||||
await legacy.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -253,7 +253,18 @@ export async function buildCollector(cfg: CollectorConfig, opts: { dbFile?: stri
|
||||
try {
|
||||
const get = async (p: string) => {
|
||||
const r = await fetch(trainer + p, { signal: AbortSignal.timeout(15_000) });
|
||||
if (!r.ok) throw new Error(`${p} → HTTP ${r.status}`);
|
||||
if (!r.ok) {
|
||||
// Surface the trainer's own error text (its handlers answer 500 JSON), so the
|
||||
// reviewer reads "no such column: kind", not just a status code.
|
||||
const detail = await r.text().then((t) => {
|
||||
try {
|
||||
return String((JSON.parse(t) as { error?: unknown }).error ?? t);
|
||||
} catch {
|
||||
return t;
|
||||
}
|
||||
}, () => "");
|
||||
throw new Error(`${p} → HTTP ${r.status}${detail ? `: ${detail.slice(0, 300)}` : ""}`);
|
||||
}
|
||||
return r.json() as Promise<Record<string, unknown>>;
|
||||
};
|
||||
const [health, readiness, versions, jobs] = await Promise.all([get("/health"), get("/readiness"), get("/versions"), get("/jobs")]);
|
||||
|
||||
@@ -71,6 +71,39 @@ export class CollectorDb {
|
||||
CREATE INDEX IF NOT EXISTS items_pending ON items (reviewed_at, received_at);
|
||||
CREATE INDEX IF NOT EXISTS items_booth ON items (booth, received_at);
|
||||
`);
|
||||
this.#migrate();
|
||||
}
|
||||
|
||||
/** Columns added after the first deploy, with the DDL that adds them to an EXISTING
|
||||
* table. `CREATE TABLE IF NOT EXISTS` above only shapes a NEW database; a volume that
|
||||
* was created by an earlier build keeps its old columns, and every query naming a new
|
||||
* one then fails ("no such column: kind" — art-docker-station, 2026-09-16: the
|
||||
* collector's /health, every ingest, and the trainer's readiness all broke on a DB
|
||||
* from before `kind`). Each entry must be addable to a populated table, i.e. nullable
|
||||
* or carrying a DEFAULT. Append here whenever a column joins the CREATE above. */
|
||||
static readonly #ADDED_COLUMNS: ReadonlyArray<readonly [name: string, ddl: string]> = [
|
||||
["kind", "TEXT NOT NULL DEFAULT 'wash'"],
|
||||
["operator_ref", "TEXT NOT NULL DEFAULT ''"],
|
||||
["operator_category_id", "TEXT NOT NULL DEFAULT ''"],
|
||||
["operator_category_name", "TEXT NOT NULL DEFAULT ''"],
|
||||
["operator_classes", "TEXT NOT NULL DEFAULT '[]'"],
|
||||
["vision_category_id", "TEXT"],
|
||||
["downgraded", "INTEGER NOT NULL DEFAULT 0"],
|
||||
];
|
||||
|
||||
/** Bring an existing `items` table up to the current column set (idempotent). */
|
||||
#migrate(): void {
|
||||
const present = new Set(
|
||||
(this.#db.prepare("PRAGMA table_info(items)").all() as { name: string }[]).map((c) => c.name),
|
||||
);
|
||||
for (const [name, ddl] of CollectorDb.#ADDED_COLUMNS) {
|
||||
if (!present.has(name)) this.#db.exec(`ALTER TABLE items ADD COLUMN ${name} ${ddl}`);
|
||||
}
|
||||
}
|
||||
|
||||
/** The current column names of `items` (for tests and diagnostics). */
|
||||
columns(): string[] {
|
||||
return (this.#db.prepare("PRAGMA table_info(items)").all() as { name: string }[]).map((c) => c.name);
|
||||
}
|
||||
|
||||
close(): void {
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import sqlite3
|
||||
import threading
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
@@ -109,3 +110,38 @@ def test_train_job_then_versions_and_report(api) -> None: # type: ignore[no-unt
|
||||
code, job2 = call("POST", "/jobs", {"kind": "evaluate", "version": "vapi"})
|
||||
wait_idle(Handler.jobs)
|
||||
assert call("GET", f"/jobs/{job2['id']}")[1]["status"] == "done"
|
||||
|
||||
|
||||
def test_a_handler_error_is_a_500_json_reply_not_a_dropped_connection(tmp_path: Path) -> None:
|
||||
"""A collector DB from before the `kind` column (art-docker-station, 2026-09-16): readiness
|
||||
raised sqlite3.OperationalError, the stdlib server printed the traceback and closed the
|
||||
socket, and the collector could only say "trainer not reachable: fetch failed". The
|
||||
handler must answer 500 JSON naming the error instead."""
|
||||
old = tmp_path / "old"
|
||||
old.mkdir()
|
||||
con = sqlite3.connect(old / "collector.sqlite")
|
||||
con.executescript(
|
||||
"CREATE TABLE items (id TEXT PRIMARY KEY, booth TEXT NOT NULL, order_ref TEXT NOT NULL,"
|
||||
" at TEXT NOT NULL, service TEXT NOT NULL, vision_class TEXT NOT NULL,"
|
||||
" vision_confidence REAL NOT NULL, image_width INTEGER NOT NULL, image_height INTEGER NOT NULL,"
|
||||
" plate_blurred INTEGER NOT NULL, image_path TEXT NOT NULL, received_at TEXT NOT NULL,"
|
||||
" review_label TEXT, reviewed_at TEXT, reviewer TEXT)"
|
||||
)
|
||||
con.commit()
|
||||
con.close()
|
||||
Handler.jobs = Jobs(old, tmp_path / "out", "https://example.invalid/pkg", "tok")
|
||||
httpd = ThreadingHTTPServer(("127.0.0.1", 0), Handler)
|
||||
threading.Thread(target=httpd.serve_forever, daemon=True).start()
|
||||
try:
|
||||
req = urllib.request.Request(f"http://127.0.0.1:{httpd.server_address[1]}/readiness")
|
||||
with pytest.raises(urllib.error.HTTPError) as ei:
|
||||
urllib.request.urlopen(req, timeout=10)
|
||||
assert ei.value.code == 500
|
||||
body = json.loads(ei.value.read())
|
||||
assert "no such column: kind" in body["error"]
|
||||
# /health does not touch the DB and still answers.
|
||||
with urllib.request.urlopen(f"http://127.0.0.1:{httpd.server_address[1]}/health", timeout=10) as r:
|
||||
assert r.status == 200
|
||||
finally:
|
||||
httpd.shutdown()
|
||||
httpd.server_close()
|
||||
|
||||
@@ -309,6 +309,26 @@ class Handler(BaseHTTPRequestHandler):
|
||||
self.wfile.write(raw)
|
||||
|
||||
def do_GET(self) -> None: # noqa: N802
|
||||
self._guarded(self._get)
|
||||
|
||||
def do_POST(self) -> None: # noqa: N802
|
||||
self._guarded(self._post)
|
||||
|
||||
def _guarded(self, handler: Any) -> None:
|
||||
"""Run a handler; an unexpected exception becomes a 500 JSON reply instead of a
|
||||
dropped connection (the stdlib server would print the traceback and close the
|
||||
socket, which the collector could only report as "fetch failed" — 2026-09-16,
|
||||
a collector DB from before the `kind` column)."""
|
||||
try:
|
||||
handler()
|
||||
except Exception as exc: # noqa: BLE001 — anything: the reply must be a reply
|
||||
sys.stderr.write(f"[trainer.serve] {self.command} {self.path}: {type(exc).__name__}: {exc}\n")
|
||||
try:
|
||||
self._json(500, {"error": f"{type(exc).__name__}: {exc}"})
|
||||
except Exception: # noqa: BLE001 — headers already sent; nothing left to do
|
||||
pass
|
||||
|
||||
def _get(self) -> None:
|
||||
path = self.path.split("?", 1)[0]
|
||||
j = self.jobs
|
||||
if path == "/health":
|
||||
@@ -336,7 +356,7 @@ class Handler(BaseHTTPRequestHandler):
|
||||
else:
|
||||
self._json(404, {"error": "not found"})
|
||||
|
||||
def do_POST(self) -> None: # noqa: N802
|
||||
def _post(self) -> None:
|
||||
if self.path.split("?", 1)[0] != "/jobs":
|
||||
self._json(404, {"error": "not found"})
|
||||
return
|
||||
|
||||
Reference in New Issue
Block a user