Fix adding engines to SQLAlchemyPanel from AsyncSession
#48
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #48 +/- ##
==========================================
- Coverage 95.73% 95.49% -0.25%
==========================================
Files 44 44
Lines 1078 1087 +9
==========================================
+ Hits 1032 1038 +6
- Misses 46 49 +3 ☔ View full report in Codecov by Sentry. |
| @@ -40,7 +41,13 @@ def after_execute(self, context: ExecutionContext, **kwargs: t.Any) -> None: | |||
| } | |||
| self.add_query(str(context.engine.url), query) | |||
|
|
|||
| async def add_engines(self, request: Request): | |||
| async def add_engines(self, request: Request): # noqa: C901 | |||
| def add_bind_to_engines(bind: t.Union[Connection, Engine]): | |||
There was a problem hiding this comment.
We can add a class method add_bind(self, bind: Connection | Engine]):, use from __future__ import annotations import to provide Python 3.8 support.
|
|
||
| if isinstance(bind, Connection): | ||
| self.engines.add(bind.engine) | ||
| binds = getattr(value, "_Session__binds", None) |
There was a problem hiding this comment.
Don't need getattr, binds = value._Session__binds is okay, default value is an empty dict.
Try value.get_bind() first and look for _Session__binds in case of UnboundExecutionError exception.
| @@ -55,13 +62,16 @@ async def add_engines(self, request: Request): | |||
| pass | |||
| else: | |||
| for value in solved_result[0].values(): | |||
| if isinstance(value, AsyncSession): | |||
| value = getattr(value, "sync_session", None) | |||
There was a problem hiding this comment.
Don't need getattr, value = value.sync_session is okay.
|
I can't wait to use this feature! When can I use it? |
Hi :з We use
AsyncSessionin our project and faced issues withSQLAlchemyPanelwhich does not supportAsyncSession.Moreover, there is no support for a
Sessionwith multiple binds.In this PR, I have attempted to fix these issues.