pierrejeambrun commented on code in PR #44208:
URL: https://github.com/apache/airflow/pull/44208#discussion_r1851719314
##########
airflow/settings.py:
##########
@@ -498,7 +498,7 @@ def configure_orm(disable_connection_pool=False,
pool_class=None):
engine = create_engine(SQL_ALCHEMY_CONN, connect_args=connect_args,
**engine_args, future=True)
async_engine = create_async_engine(SQL_ALCHEMY_CONN_ASYNC, future=True)
- create_async_session = sessionmaker(
+ session_maker_async = sessionmaker(
Review Comment:
Your name is more explicit, and I understand why legacy names can feel
confusing and this one could be better. (We call a `session_maker` `Session` oO
=> but that's actually a factory for session, so from the client code
perspective we are constructing a session and we think we are manipulating a
`Session` class for instantiation, that's especially true for the threadlocal
registry `scoped_session`, just create a Session but actually things happen
under the hood, we are not just instantiating a session and we don't ask people
to explicitely use a factory, just the normal `Session`, that's how sqlalchemy
docs and many other documents online recommend to do it.)
Anyway, for consistency, I recommend using `AsyncSession`. (same way as
`Session` and `NonScopedSession` above).
In the user code this it just becomes `session = AsyncSession()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
