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`.)
   
   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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to