Re: [PR] Correctly shutdown async sessions on exit. [airflow]

["kaxil (via GitHub)" <gi...@apache.org>]

kaxil commented on code in PR #73838:
URL: https://github.com/apache/airflow/pull/73838#discussion_r4154122855


##########
airflow-core/src/airflow/settings.py:
##########
@@ -670,6 +664,16 @@ def dispose_orm(do_log: bool = True):
         AsyncSession = None
 
 
+async def dispose_async_orm() -> None:
+    """Dispose the async pool on its owning loop after closing its sessions."""
+    global async_engine, AsyncSession
+
+    if async_engine is not None:
+        await async_engine.dispose()
+    async_engine = None

Review Comment:
   One more data point for this thread. The same `MissingGreenlet` log is still reachable through `InProcessExecutionAPI` (triggerer, dag-processor, `dag.test`): `_shutdown_loop` only closes the execution app's own lifespan, and since #73403 those routes use async sessions, so the atexit `dispose_orm` still sync-disposes a pool with live connections. The obvious follow-up is to await `dispose_async_orm` there as well, but with the reset in place, tearing down an in-process transport would set `settings.AsyncSession` to None for anything else in that process, and the next `create_session_async()` would call `None()`. Dropping the reset would also let the `skip_db_tests` autospec in the api_fastapi conftest go. Right now that guard keys on `pytestconfig.getoption`, so it misses `breeze shell --skip-db-tests`, which only sets `_AIRFLOW_SKIP_DB_TESTS` (the plugin's own `skip_db_tests` checks both).



-- 
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-unsubscribe@airflow.apache.org

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