fix: handle pre-begun transactions and custom PK column names #5

Merged
i.bahlai merged 1 commit from fix/session-transaction-and-pk-lookup into main 2026-04-04 17:45:26 +00:00

Summary

Two bugs causing 500 errors in print-estimate-api E2E tests:

Bug 1: Session transaction conflict

  • Symptom: InvalidRequestError: A transaction is already begun on this Session.
  • Cause: create/update/remove repos call session.begin(), but when the session factory already starts a transaction (e.g. sessionmaker.begin() pattern used in peapi), this raises an error.
  • Fix: New async_transaction() / sync_transaction() helpers that check session.in_transaction() and use begin_nested() (savepoint) when a transaction is already active.

Bug 2: Primary key attribute lookup

  • Symptom: AttributeError: material_profile_id
  • Cause: get_primary_key() returned SQL column names instead of Python attribute names. When models use id: PrimaryKeyType = PrimaryKey("material_profile_id"), the column name != attribute name.
  • Fix: Use SQLAlchemy mapper inspection (sa_inspect) to resolve column name → Python attribute name mapping.

Test plan

  • 106/106 tests pass (including 7 new tests)
  • New tests: TestAsyncPreBegunTransaction (4 tests), TestSyncPreBegunTransaction (2 tests), test_custom_column_name_returns_attribute_name (1 test)
  • Ruff lint + format clean
  • CI pipeline pass on PR

Version bumped to 1.8.2.

## Summary Two bugs causing 500 errors in print-estimate-api E2E tests: ### Bug 1: Session transaction conflict - **Symptom:** `InvalidRequestError: A transaction is already begun on this Session.` - **Cause:** `create/update/remove` repos call `session.begin()`, but when the session factory already starts a transaction (e.g. `sessionmaker.begin()` pattern used in peapi), this raises an error. - **Fix:** New `async_transaction()` / `sync_transaction()` helpers that check `session.in_transaction()` and use `begin_nested()` (savepoint) when a transaction is already active. ### Bug 2: Primary key attribute lookup - **Symptom:** `AttributeError: material_profile_id` - **Cause:** `get_primary_key()` returned SQL column names instead of Python attribute names. When models use `id: PrimaryKeyType = PrimaryKey("material_profile_id")`, the column name != attribute name. - **Fix:** Use SQLAlchemy mapper inspection (`sa_inspect`) to resolve column name → Python attribute name mapping. ## Test plan - [x] 106/106 tests pass (including 7 new tests) - [x] New tests: `TestAsyncPreBegunTransaction` (4 tests), `TestSyncPreBegunTransaction` (2 tests), `test_custom_column_name_returns_attribute_name` (1 test) - [x] Ruff lint + format clean - [ ] CI pipeline pass on PR Version bumped to 1.8.2.
fix: handle pre-begun transactions and custom PK column names
All checks were successful
CI Pipeline - Development / Run Tests (pull_request) Successful in 1m31s
CI Pipeline - Development / Code Quality Check (pull_request) Successful in 1m44s
29ec39bbe0
Two bugs fixed:

1. Session transaction conflict: When the session factory already
   starts a transaction (e.g. `sessionmaker.begin()`), the repo's
   internal `session.begin()` raised `InvalidRequestError: A
   transaction is already begun`. Fix: use `begin_nested()` (savepoint)
   when a transaction is already active. Affects create, update, and
   remove in both async and sync repos.

2. Primary key attribute lookup: `get_primary_key()` returned SQL
   column names (e.g. `material_profile_id`) instead of Python
   attribute names (e.g. `id`). This caused `AttributeError` in
   `get_entity()` and `remove()` when models use custom PK column
   names via `sa_column=Column("custom_name", ...)`. Fix: use
   SQLAlchemy mapper inspection to resolve column→attribute mapping.

Bump version to 1.8.2.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Sign in to join this conversation.
No reviewers
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
CobraPack/repository!5
No description provided.