Skip to content

Comments

fix(python): commit transaction after creating lamp#407

Merged
davideme merged 1 commit intomainfrom
claude/keen-mendeleev
Feb 22, 2026
Merged

fix(python): commit transaction after creating lamp#407
davideme merged 1 commit intomainfrom
claude/keen-mendeleev

Conversation

@davideme
Copy link
Owner

Summary

  • PostgresLampRepository.create() was calling flush() but never commit(), so the INSERT was sent within the open transaction but rolled back when the session closed at the end of the request
  • Subsequent GET requests from a new session correctly returned 404 since the lamp was never persisted
  • update and delete both had commit()create was simply missing it

Test plan

  • POST /v1/lamps followed by GET /v1/lamps/{lampId} returns 200 (was 404)
  • Benchmark precheck no longer fails with repeated 404s
  • Existing unit/integration tests continue to pass

🤖 Generated with Claude Code

The create method was only calling flush() which sends SQL within the
open transaction but never committed it. When the session closed at the
end of the POST request, SQLAlchemy rolled back the uncommitted
transaction, so the lamp was never persisted. Subsequent GET requests
correctly returned 404.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 22, 2026 17:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes persistence of newly created lamps when using the PostgreSQL repository by ensuring the INSERT is committed so subsequent requests/sessions can read the row.

Changes:

  • Add an explicit commit() to PostgresLampRepository.create() after flush()

Comment on lines 58 to 61
self._session.add(db_lamp)
await self._session.flush()
await self._session.commit()
await self._session.refresh(db_lamp)
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test coverage gap: the existing PostgresLampRepository integration tests exercise create() and then get() within the same SQLAlchemy session/transaction, which would still pass even if create() only did flush() (the root cause described in the PR). To prevent regressions, add a test that creates a lamp, closes the session/request, and then uses a new session/repository instance to get() the lamp and assert it is found.

Copilot generated this review using guidance from repository custom instructions.
@davideme davideme merged commit a87e535 into main Feb 22, 2026
7 checks passed
@davideme davideme deleted the claude/keen-mendeleev branch February 22, 2026 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant