From 75a64b3062dabdb18e8beb05df53029e33b9cc8d Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Mon, 11 May 2026 15:57:19 -0400 Subject: [PATCH] Address comments --- UPDATING.md | 18 ++++++++++++++++++ superset/config.py | 7 +++++++ superset/engines/manager.py | 7 ++----- superset/extensions/engine_manager.py | 7 +++++-- superset/models/core.py | 15 +++++++++++++++ 5 files changed, 47 insertions(+), 7 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 527c32f846b..3061e5e0851 100644 --- a/UPDATING.md +++ b/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: diff --git a/superset/config.py b/superset/config.py index a2df28e1019..1b0e6739100 100644 --- a/superset/config.py +++ b/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 diff --git a/superset/engines/manager.py b/superset/engines/manager.py index 52597a260ad..7ebc6d8dafb 100644 --- a/superset/engines/manager.py +++ b/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] diff --git a/superset/extensions/engine_manager.py b/superset/extensions/engine_manager.py index d775d071f16..74c18f4ee66 100644 --- a/superset/extensions/engine_manager.py +++ b/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, diff --git a/superset/models/core.py b/superset/models/core.py index 59626e083a4..c19809c7588 100755 --- a/superset/models/core.py +++ b/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