Skip to content

fix: disable implicit transactions in sqlite #102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
QueryPlanner merged 2 commits into from
Jun 1, 2026
Merged

fix: disable implicit transactions in sqlite #102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
250a3a7
fix: disable implicit transactions in sqlite
QueryPlanner Jun 1, 2026
0c7f649
fix: use explicit SQL for transaction control
QueryPlanner Jun 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/blacki/storage/sqlite.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ async def create_connection(
path = Path(db_path)
path.parent.mkdir(parents=True, exist_ok=True)

conn = await aiosqlite.connect(path)
conn = await aiosqlite.connect(path, isolation_level=None)
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Jun 1, 2026

Choose a reason for hiding this comment

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

critical

Setting isolation_level=None puts the connection into autocommit mode and disables Python's automatic transaction management. While this is the correct way to handle manual transactions, it has a critical side effect: calling connection.commit() and connection.rollback() on the connection object becomes a silent no-op in Python's sqlite3 (and thus aiosqlite).

In src/blacki/workouts/storage.py (line 116), create_session starts a manual transaction with BEGIN but attempts to commit/rollback using await self._conn.commit() and await self._conn.rollback(). With isolation_level=None, these calls will do nothing, leaving the transaction open indefinitely and causing subsequent operations to fail with database locks or transaction errors.

To fix this, you must update create_session to use explicit SQL statements for committing and rolling back:

await self._conn.execute("COMMIT")
await self._conn.execute("ROLLBACK")
Suggested change
conn = await aiosqlite.connect(path, isolation_level=None)
# isolation_level=None disables implicit transactions (autocommit mode).
# Note: Use execute("COMMIT") / execute("ROLLBACK") instead of conn.commit() / conn.rollback().
conn = await aiosqlite.connect(path, isolation_level=None)

conn.row_factory = aiosqlite.Row

await conn.execute("PRAGMA journal_mode=WAL")
await conn.execute(f"PRAGMA busy_timeout={busy_timeout_ms}")
await conn.execute("PRAGMA foreign_keys=ON")

logger.info("SQLite connection opened: %s (WAL mode)", path)
logger.info("SQLite connection opened: %s (WAL mode, autocommit)", path)
return conn


Expand Down
4 changes: 2 additions & 2 deletions src/blacki/workouts/storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,10 @@ async def create_session(self, session: WorkoutSession) -> int:
exercise.notes,
),
)
await self._conn.commit()
await self._conn.execute("COMMIT")
return sid
except Exception:
await self._conn.rollback()
await self._conn.execute("ROLLBACK")
raise

async def add_exercise(self, session_id: int, exercise: WorkoutExercise) -> int:
Expand Down
2 changes: 1 addition & 1 deletion tests/calories/test_storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
@pytest.fixture
async def conn():
"""Create an in-memory SQLite connection for testing."""
conn = await aiosqlite.connect(":memory:")
conn = await aiosqlite.connect(":memory:", isolation_level=None)
conn.row_factory = aiosqlite.Row
yield conn
await conn.close()
Expand Down
2 changes: 1 addition & 1 deletion tests/reminders/test_storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
@pytest.fixture
async def conn():
"""Create an in-memory SQLite connection for testing."""
conn = await aiosqlite.connect(":memory:")
conn = await aiosqlite.connect(":memory:", isolation_level=None)
conn.row_factory = aiosqlite.Row
yield conn
await conn.close()
Expand Down
2 changes: 1 addition & 1 deletion tests/storage/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ async def _create_tables(self) -> None:
@pytest.fixture
async def conn():
"""Create an in-memory SQLite connection for testing."""
conn = await aiosqlite.connect(":memory:")
conn = await aiosqlite.connect(":memory:", isolation_level=None)
conn.row_factory = aiosqlite.Row
yield conn
await conn.close()
Expand Down
2 changes: 1 addition & 1 deletion tests/storage/test_sqlite.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ async def test_close_connection_with_memory_db(self) -> None:
"""Should close an in-memory connection."""
import aiosqlite

conn = await aiosqlite.connect(":memory:")
conn = await aiosqlite.connect(":memory:", isolation_level=None)

await close_connection(conn)

Expand Down
6 changes: 3 additions & 3 deletions tests/test_container.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def teardown_method(self) -> None:
@pytest.mark.asyncio
async def test_creates_container_from_connection(self) -> None:
"""Should create and set container from existing connection."""
conn = await aiosqlite.connect(":memory:")
conn = await aiosqlite.connect(":memory:", isolation_level=None)
try:
container = set_container_from_connection(conn)

Expand All @@ -80,7 +80,7 @@ async def test_creates_container_from_connection(self) -> None:
@pytest.mark.asyncio
async def test_creates_container_with_custom_lock(self) -> None:
"""Should use provided lock."""
conn = await aiosqlite.connect(":memory:")
conn = await aiosqlite.connect(":memory:", isolation_level=None)
try:
custom_lock = asyncio.Lock()
container = set_container_from_connection(conn, lock=custom_lock)
Expand Down Expand Up @@ -150,7 +150,7 @@ def teardown_method(self) -> None:
@pytest.fixture
async def conn(self):
"""Create an in-memory SQLite connection for testing."""
conn = await aiosqlite.connect(":memory:")
conn = await aiosqlite.connect(":memory:", isolation_level=None)
yield conn
await conn.close()

Expand Down
2 changes: 1 addition & 1 deletion tests/test_preferences.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
@pytest.fixture
async def conn():
"""Create an in-memory SQLite connection for testing."""
conn = await aiosqlite.connect(":memory:")
conn = await aiosqlite.connect(":memory:", isolation_level=None)
conn.row_factory = aiosqlite.Row
yield conn
await conn.close()
Expand Down
25 changes: 15 additions & 10 deletions tests/workouts/test_storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
@pytest.fixture
async def conn():
"""Create an in-memory SQLite connection for testing."""
conn = await aiosqlite.connect(":memory:")
conn = await aiosqlite.connect(":memory:", isolation_level=None)
conn.row_factory = aiosqlite.Row
yield conn
await conn.close()
Expand Down Expand Up @@ -586,8 +586,6 @@ async def test_create_session_raises_when_lastrowid_none(self, conn, lock) -> No
mock_cursor = AsyncMock()
mock_cursor.lastrowid = None
mock_conn.execute.return_value = mock_cursor
mock_conn.commit = AsyncMock()
mock_conn.rollback = AsyncMock()

storage = SqliteWorkoutStorage(mock_conn, lock)
storage._schema_ready = True
Expand All @@ -605,7 +603,9 @@ async def test_create_session_raises_when_lastrowid_none(self, conn, lock) -> No
):
await storage.create_session(session)

mock_conn.rollback.assert_called_once()
mock_conn.execute.assert_called()
call_args = [call[0][0] for call in mock_conn.execute.call_args_list]
assert "ROLLBACK" in call_args

@pytest.mark.asyncio
async def test_create_session_rollback_on_exception(self, conn, lock) -> None:
Expand All @@ -615,11 +615,15 @@ async def test_create_session_rollback_on_exception(self, conn, lock) -> None:
import aiosqlite

mock_conn = AsyncMock(spec=aiosqlite.Connection)
mock_cursor = AsyncMock()
mock_cursor.lastrowid = 1
mock_conn.execute.return_value = mock_cursor
mock_conn.commit = AsyncMock(side_effect=Exception("commit failed"))
mock_conn.rollback = AsyncMock()

async def execute_side_effect(query, *args):
if "COMMIT" in query:
raise Exception("commit failed")
mock_cursor = AsyncMock()
mock_cursor.lastrowid = 1
return mock_cursor

mock_conn.execute.side_effect = execute_side_effect

storage = SqliteWorkoutStorage(mock_conn, lock)
storage._schema_ready = True
Expand All @@ -635,4 +639,5 @@ async def test_create_session_rollback_on_exception(self, conn, lock) -> None:
with pytest.raises(Exception, match="commit failed"):
await storage.create_session(session)

mock_conn.rollback.assert_called_once()
call_args = [call[0][0] for call in mock_conn.execute.call_args_list]
assert "ROLLBACK" in call_args
Loading