Skip to content

Commit d57c79a

Browse files
authored
Merge pull request #32 from febus982/use_asyncio_run
Fix: Make sure async scoped session is removed when no loop available
2 parents 1367851 + b63c8f6 commit d57c79a

File tree

3 files changed

+27
-23
lines changed

3 files changed

+27
-23
lines changed

docs/lifecycle.md

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
### Bind manager
44

55
The `SQLAlchemyBindManager` object holds all the SQLAlchemy Engines, which
6-
are supposed to be global objects. Therefore it should be created on application
6+
are supposed to be global objects, therefore it should be created on application
77
startup and be globally accessible.
88

99
From SQLAlchemy documentation:
@@ -17,22 +17,20 @@ From SQLAlchemy documentation:
1717
recommends we create `Session` object at the beginning of a logical operation where
1818
database access is potentially anticipated.
1919

20-
The repository starts a `Session` for each _operation_, in order to maintain isolation.
21-
This means you can create a repository object almost whenever you want.
20+
The repository is not built for parallel execution, it keeps a `Session` object scoped to
21+
its lifecycle to avoid unnecessary queries, and executes a transaction for each _operation_
22+
to maintain isolation. This means you can create a repository object almost whenever you want,
23+
as long as you don't run parallel operations.
2224

23-
/// details | There two exceptions: creating repositories in global variables or concurrent asyncio tasks
24-
type: warning
25-
The repository creates and maintain the lifecycle of a session object to avoid
26-
emitting unnecessary queries to refresh the model state on new session.
25+
The session in the repository is not thread safe and [is not safe on concurrent asyncio tasks](https://docs.sqlalchemy.org/en/20/orm/session_basics.html#is-the-session-thread-safe-is-asyncsession-safe-to-share-in-concurrent-tasks)
26+
therefore the repository has the same limitations.
2727

28-
The session is not thread safe, therefore the repository is not thread safe as well.
28+
This means:
2929

30-
Check the [Notes on multithreaded applications](/manager/session/#note-on-multithreaded-applications)
31-
32-
The `AsyncSession` [is not safe on concurrent asyncio tasks](https://docs.sqlalchemy.org/en/20/orm/session_basics.html#is-the-session-thread-safe-is-asyncsession-safe-to-share-in-concurrent-tasks),
33-
therefore the same repository instance can't be used in multiple asyncio tasks like
34-
when using `asyncio.gather()`
35-
///
30+
* Do not assign a repository object to a global variable
31+
(Check the [Notes on multithreaded applications](/manager/session/#note-on-multithreaded-applications))
32+
* Do not share a repository instance in multiple threads or processes (e.g. using `asyncio.to_thread`).
33+
* Do not use the same repository in concurrent asyncio task (e.g. using `asyncio.gather`)
3634

3735
Even using multiple repository instances will work fine, however as they will have completely
3836
different sessions, it's likely that the second repository will fire additional SELECT queries
@@ -71,8 +69,8 @@ update_my_model()
7169
```
7270
///
7371

74-
The recommendation is of course to use the same repository instance, where possible,
75-
and structure your code in a way to match the single repository instance approach.
72+
The recommendation is of course to try to write your application in a way you cna use
73+
a single repository instance, where possible.
7674

7775
For example a strategy similar to this would be optimal, if possible:
7876

sqlalchemy_bind_manager/_transaction_handler.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from asyncio import get_event_loop
1+
import asyncio
22
from contextlib import asynccontextmanager, contextmanager
33
from typing import AsyncIterator, Iterator
44
from uuid import uuid4
@@ -74,11 +74,14 @@ def __del__(self):
7474
if not getattr(self, "_session_class", None):
7575
return
7676

77-
loop = get_event_loop()
78-
if loop.is_running():
79-
loop.create_task(self._session_class.remove())
80-
else:
81-
loop.run_until_complete(self._session_class.remove())
77+
try:
78+
loop = asyncio.get_event_loop()
79+
if loop.is_running():
80+
loop.create_task(self._session_class.remove())
81+
else:
82+
loop.run_until_complete(self._session_class.remove())
83+
except RuntimeError:
84+
asyncio.run(self._session_class.remove())
8285

8386
@asynccontextmanager
8487
async def get_session(self, read_only: bool = False) -> AsyncIterator[AsyncSession]:

tests/transaction_handler/async_/test_session_lifecycle.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,13 @@ def test_session_is_destroyed_on_cleanup_if_loop_is_not_running(sa_manager):
3030
uow._session_class,
3131
"remove",
3232
wraps=original_session_close,
33-
) as mocked_close:
33+
) as mocked_close, patch(
34+
"asyncio.get_event_loop", side_effect=RuntimeError()
35+
) as mocked_get_event_loop:
3436
# This should trigger the garbage collector and close the session
3537
uow = None
3638

39+
mocked_get_event_loop.assert_called_once()
3740
mocked_close.assert_called_once()
3841

3942

0 commit comments

Comments
 (0)