Address comments

UPDATING.md

@@ -24,6 +24,24 @@ assists people when migrating to a new version.
 
 ## Next
 
+### `SSH_TUNNEL_MANAGER_CLASS` replaced by `ENGINE_MANAGER_CLASS`
+
+The `SSH_TUNNEL_MANAGER_CLASS` config setting, the `superset.extensions.ssh` module (containing `SSHManager` and `SSHManagerFactory`), and the `ssh_manager_factory` extension singleton have been removed. SQLAlchemy engine creation — including SSH tunnel construction and URL rewriting — is now centralized in `EngineManager` (`superset/engines/manager.py`), wired up via `EngineManagerExtension` (`superset/extensions/engine_manager.py`).
+
+A new config setting, `ENGINE_MANAGER_CLASS` (default: `"superset.engines.manager.EngineManager"`), replaces `SSH_TUNNEL_MANAGER_CLASS` as the customization hook. Deployments that previously subclassed `SSHManager` (e.g. for bastion routing, audit logging, host-key policy, or custom credential handling) should subclass `EngineManager` instead and set `ENGINE_MANAGER_CLASS` to the dotted path of the subclass. Override the relevant methods:
+
+| Old `SSHManager` method | New override point on `EngineManager` |
+|---|---|
+| `__init__(app)` reading `SSH_TUNNEL_*` configs | `__init__` — the same `SSH_TUNNEL_LOCAL_BIND_ADDRESS`, `SSH_TUNNEL_TIMEOUT_SEC`, and `SSH_TUNNEL_PACKET_TIMEOUT_SEC` configs are still loaded by `EngineManagerExtension.init_app` and passed in |
+| `create_tunnel(ssh_tunnel, uri)` | `_get_tunnel_kwargs(ssh_tunnel, uri)` for parameter construction and `_create_tunnel(ssh_tunnel, uri)` for the `sshtunnel.open_tunnel` + `start()` call |
+| `build_sqla_url(url, server)` | Inlined in `get_engine` as `uri.set(host=tunnel.local_bind_address[0], port=tunnel.local_bind_port)` |
+
+**Behavioral note:** the old `SSHManager.create_tunnel` passed `debug_level=logging.getLogger("flask_appbuilder").level` to `sshtunnel.open_tunnel`. The new `_get_tunnel_kwargs` does not. Subclasses relying on that should add it back in their override.
+
+### `Database.get_sqla_engine(nullpool=...)` deprecated
+
+The `nullpool` keyword argument to `Database.get_sqla_engine` is deprecated and ignored — the engine manager always uses `NullPool`. The kwarg is still accepted (with a `DeprecationWarning`) so external callers passing `nullpool=False` won't fail with `TypeError`, but the resulting engine will use `NullPool` regardless. Remove the argument from your callers; it will be deleted in a future release.
+
 ### Granular Export Controls
 
 A new feature flag `GRANULAR_EXPORT_CONTROLS` introduces three fine-grained permissions that replace the legacy `can_csv` permission:

superset/config.py

@@ -1725,6 +1725,13 @@ def engine_context_manager(  # pylint: disable=unused-argument
 
 ENGINE_CONTEXT_MANAGER: EngineContextManager = engine_context_manager
 
+# The class used to manage SQLAlchemy engine creation, including SSH tunnels
+# and connection details. Deployments that need custom behavior (e.g. bastion
+# routing, audit logging, host-key policy, custom credential handling) can
+# subclass `superset.engines.manager.EngineManager` and point this setting at
+# the subclass.
+ENGINE_MANAGER_CLASS = "superset.engines.manager.EngineManager"
+
 # A callable that allows altering the database connection URL and params
 # on the fly, at runtime. This allows for things like impersonation or
 # arbitrary logic. For instance you can wire different users to

superset/engines/manager.py

@@ -98,8 +98,8 @@ class EngineManager:
         user_id: int | None,
     ) -> tuple[URL, dict[str, Any]]:
         """Build SQLAlchemy URI and kwargs before engine creation."""
+        from superset import is_feature_enabled
         from superset.extensions import security_manager
-        from superset.utils.feature_flag_manager import FeatureFlagManager
 
         uri = make_url_safe(database.sqlalchemy_uri_decrypted)
         extra = database.get_extra(source)
@@ -115,10 +115,7 @@ class EngineManager:
         )
 
         username = database.get_effective_user(uri)
-        feature_flag_manager = FeatureFlagManager()
-        if username and feature_flag_manager.is_feature_enabled(
-            "IMPERSONATE_WITH_EMAIL_PREFIX"
-        ):
+        if username and is_feature_enabled("IMPERSONATE_WITH_EMAIL_PREFIX"):
             user = security_manager.find_user(username=username)
             if user and user.email and "@" in user.email:
                 username = user.email.split("@")[0]

superset/extensions/engine_manager.py

@@ -21,6 +21,7 @@ from datetime import timedelta
 from flask import Flask
 
 from superset.engines.manager import EngineManager
+from superset.utils.class_utils import load_class_from_name
 
 logger = logging.getLogger(__name__)
 
@@ -43,8 +44,10 @@ class EngineManagerExtension:
         tunnel_timeout = timedelta(seconds=app.config["SSH_TUNNEL_TIMEOUT_SEC"])
         ssh_timeout = timedelta(seconds=app.config["SSH_TUNNEL_PACKET_TIMEOUT_SEC"])
 
-        # Create the engine manager
-        self.engine_manager = EngineManager(
+        engine_manager_class: type[EngineManager] = load_class_from_name(
+            app.config["ENGINE_MANAGER_CLASS"]
+        )
+        self.engine_manager = engine_manager_class(
             engine_context_manager,
             db_connection_mutator,
             local_bind_address,

superset/models/core.py

@@ -424,6 +424,7 @@ class Database(CoreDatabase, AuditMixinNullable, ImportExportMixin):  # pylint:
         catalog: str | None = None,
         schema: str | None = None,
         source: utils.QuerySource | None = None,
+        nullpool: bool | None = None,
     ) -> Iterator[Engine]:
         """
         Context manager for a SQLAlchemy engine.
@@ -431,7 +432,21 @@ class Database(CoreDatabase, AuditMixinNullable, ImportExportMixin):  # pylint:
         This method will return a context manager for a SQLAlchemy engine. The engine
         manager handles engine creation, SSH tunnels, and connection details in a
         centralized place.
+
+        The ``nullpool`` argument is deprecated and ignored — the engine manager
+        always uses ``NullPool``. It is kept temporarily for backwards compatibility
+        with external callers and will be removed in a future release.
         """
+        if nullpool is not None:
+            import warnings
+
+            warnings.warn(
+                "The `nullpool` argument to `Database.get_sqla_engine` is "
+                "deprecated and ignored; the engine manager always uses NullPool.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
         # Import here to avoid circular imports
         from superset.extensions import engine_manager_extension