sadpandajoe commented on code in PR #40237:
URL: https://github.com/apache/superset/pull/40237#discussion_r3291584009
##########
superset/models/core.py:
##########
@@ -565,10 +571,29 @@ def _get_sqla_engine( # pylint: disable=too-many-locals
# noqa: C901
security_manager,
source,
)
+ # Per-process engine cache (#27897). SQLAlchemy expects
``create_engine``
+ # to be called once per process per URL so its connection pool can do
+ # its job. Recreating the engine every call defeats the pool that
+ # operators configure via ``DB_CONNECTION_MUTATOR`` (e.g. duckdb with a
+ # size-1 queue). Skip the cache when ``nullpool`` is True — those
+ # engines are intentionally poolless and there's nothing to reuse.
+ cache_key: tuple[Any, ...] | None = None
+ if not nullpool:
Review Comment:
I think the cache is no longer engaged now. The new code is gated by `if
not nullpool:` but every callsite of `get_sqla_engine`/`_get_sqla_engine in
superset/` accepts the `default nullpool=True`. The` nullpool=True` branch then
forces `poolclass=NullPool` at :521, which is the exact behavior the issue says
defeats `DB_CONNECTION_MUTATOR`'s pool.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
