[Arch] Multi-step database writes are not transactional #57

New issue

Closed

opened 2026-07-01 07:26:49 +02:00 by bullitt · 1 comment

bullitt commented

2026-07-01 07:26:49 +02:00

Owner

Severity: High | Category: Data

db.get_db() wraps a single connection only; upsert_shipment → add_event → finalize_shipment each open/commit their own connection independently. A crash mid-sequence leaves a shipment without its event.

Recommendation: Add a db.transaction() context manager threading one connection through the call sequence; use it in ingest.py:353-374 and both scheduler.py scrape-result paths.

Refactoring safety: Add a test that forces a mid-sequence exception and asserts neither write persisted.

Full detail: docs/architecture-review.md § C + § H.7

**Severity:** High | **Category:** Data `db.get_db()` wraps a single connection only; `upsert_shipment` → `add_event` → `finalize_shipment` each open/commit their own connection independently. A crash mid-sequence leaves a shipment without its event. **Recommendation:** Add a `db.transaction()` context manager threading one connection through the call sequence; use it in `ingest.py:353-374` and both `scheduler.py` scrape-result paths. **Refactoring safety:** Add a test that forces a mid-sequence exception and asserts neither write persisted. Full detail: `docs/architecture-review.md` § C + § H.7

bullitt added the

arch

label

2026-07-01 07:27:46 +02:00

bullitt referenced this issue from a commit

2026-07-01 07:55:49 +02:00

fix(db): make upsert_shipment + add_event atomic via db.transaction() (fixes #57)

bullitt commented

2026-07-01 07:56:24 +02:00

Author

Owner

Fixed on branch architecture-rework, commit 519f712: added db.transaction() (explicit BEGIN IMMEDIATE/COMMIT/ROLLBACK) and threaded an optional conn= parameter through get_shipment/find_shipment/create_shipment/update_shipment/upsert_shipment/add_event. ingest.py's email pipeline now wraps upsert_shipment + add_event (incl. the activities-loop events) in one transaction.

Added test_upsert_and_event_write_is_atomic_on_failure (forces db.add_event to raise mid-sequence, asserts the shipment write rolled back too) — confirmed it fails on the pre-fix code by temporarily reverting, then passes after. Full suite: 240 passed at the time of this commit.

Fixed on branch `architecture-rework`, commit 519f712: added `db.transaction()` (explicit `BEGIN IMMEDIATE`/`COMMIT`/`ROLLBACK`) and threaded an optional `conn=` parameter through `get_shipment`/`find_shipment`/`create_shipment`/`update_shipment`/`upsert_shipment`/`add_event`. `ingest.py`'s email pipeline now wraps `upsert_shipment` + `add_event` (incl. the activities-loop events) in one transaction. Added `test_upsert_and_event_write_is_atomic_on_failure` (forces `db.add_event` to raise mid-sequence, asserts the shipment write rolled back too) — confirmed it fails on the pre-fix code by temporarily reverting, then passes after. Full suite: 240 passed at the time of this commit.

bullitt closed this issue

2026-07-01 07:56:36 +02:00

No labels

arch

harness

security

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

bullitt/trackbox#57

No description provided.

Rows
Columns