diff --git a/UPDATING.md b/UPDATING.md index 64787158583..82525b7a81a 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -74,31 +74,6 @@ for downstream analysis but is a visible change for anyone who relied on the formatted text in those files. The rendered email body (the only place the formatting is intended for) is unaffected. -### SQLAlchemy bumped to 2.0, flask-sqlalchemy to 3.1.1 - -Superset's core ORM dependencies move from SQLAlchemy 1.4 to 2.0 and -flask-sqlalchemy `<3.0` to 3.1.1, completing the migration tracked in -[discussion #40273](https://github.com/apache/superset/discussions/40273). - -**Custom `db_engine_specs`, plugins, or extensions that import SQLAlchemy -internals directly** should review the -[SQLAlchemy 1.4-to-2.0 migration guide](https://docs.sqlalchemy.org/en/20/changelog/migration_20.html) -for API changes that affect them — most 1.4 code already runs unmodified -under 2.0's compatibility mode, but patterns like `Engine.execute()`, -string-keyed `Row` access, and `MetaData(bind=)` are removed outright. - -**Several optional DB-connector extras remain capped below their -SQLAlchemy-2.0-only releases**, either because that bump is a separate -follow-up ([#42891](https://github.com/apache/superset/pull/42891): dremio, -exasol, firebird, redshift, risingwave) or because the upstream dialect -package has no SQLAlchemy 2.0 support yet at all (aurora-data-api, d1, -kusto, solr; ocient's 2.0 compatibility is unverified). Installing one of -these extras continues to pull a SQLAlchemy-1.4-line version of that -dialect; each package's constraint in `pyproject.toml` documents why. - -No application-level configuration changes are required for deployments -that don't touch SQLAlchemy directly. - ### Soft delete is on by default, and purging is live `SOFT_DELETE` now ships **on** (`DEFAULT_FEATURE_FLAGS`), so deleting a diff --git a/pyproject.toml b/pyproject.toml index d82adc359c5..9731f6b0127 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -60,11 +60,15 @@ dependencies = [ "flask-login>=0.6.0, < 1.0", "flask-migrate>=4.1.0, <5.0", "flask-session>=0.4.0, <1.0", - # Bumped to 3.1.1 alongside the SQLAlchemy 2.0 core bump (discussion - # #40273, step 6), which resolves the session/app-context handling - # across Celery task boundaries that previously blocked this (see - # PR #42542). - "flask-sqlalchemy>=3.1.1, <4.0", + # Pinned explicitly below 3.0: 3.0.5 resolves without conflict and + # supports both SQLAlchemy 1.4 and 2.0, but real CI runs surfaced a + # structural incompatibility with Superset's current session/app-context + # handling across Celery task boundaries (see PR #42542) -- widespread + # "NoneType has no attribute X" failures and MySQL lock-wait timeouts, + # not just a connection-pool quirk. Needs dedicated investigation, not a + # driver-compat-prep bump; revisit alongside the actual SQLAlchemy 2.0 + # core bump (discussion #40273, step 6). + "flask-sqlalchemy>=2.5.1, <4.0", "flask-wtf>=1.3.0, <2.0", "geopy", "greenlet<=3.5.4, >=3.5.4", @@ -111,7 +115,7 @@ dependencies = [ "sshtunnel>=0.4.0, <0.5", "simplejson>=4.1.1", "slack_sdk>=3.43.0, <4", - "sqlalchemy>=2.0.0, <2.1", + "sqlalchemy>=1.4.43, <2", # 1.4.43 adds the python-oracledb (oracle+oracledb) dialect "sqlalchemy-continuum>=1.6.0, <2.0.0", "sqlalchemy-utils>=0.42.1, <0.43", # expanding lowerbound to work with pydoris "sqlglot>=30.16.0, <31", # 30.16.0 adds Trino inline UDF IF/CASE routine statement parsing @@ -160,10 +164,11 @@ databricks = [ datafusion = ["flightsql-dbapi>=0.2.2, <0.3"] db2 = ["ibm-db-sa<=0.4.4, >=0.4.4"] denodo = ["denodo-sqlalchemy>=2.0.5,<2.1.0"] -# sqlalchemy-dremio 3.0.5+ hard-pins sqlalchemy~=2.0.41, dropping 1.4. -# Widened now that Superset's own SQLAlchemy 2.0 core bump has landed -# (discussion #40273). -dremio = ["sqlalchemy-dremio>=3.0.5, <4"] +# sqlalchemy-dremio 3.0.5+ hard-pins sqlalchemy~=2.0.41, dropping 1.4; 3.0.4 +# is the last dual-compat release. Capped below 3.0.5 for now; widen back to +# <4 in lockstep with Superset's own SQLAlchemy 2.0 core bump (discussion +# #40273), not before. +dremio = ["sqlalchemy-dremio>=1.2.1, <3.0.5"] # <2 was an artificial ceiling; upstream has no SQLAlchemy version cap and # 1.1.10 already supports SQLAlchemy 2.0 (added `import_dbapi` in 1.1.7). drill = ["sqlalchemy-drill>=1.1.10, <3"] @@ -176,9 +181,10 @@ dynamodb = ["pydynamodb>=0.8.2"] solr = ["sqlalchemy-solr>=0.2.4.3"] elasticsearch = ["elasticsearch-dbapi>=0.2.13, <0.3.0"] # sqlalchemy-exasol cuts hard from SQLAlchemy 1.4-only (<6.0.0) to 2.0-only -# (>=6.0.0) with no dual-compat release. Bumped now that Superset's own -# SQLAlchemy 2.0 core bump has landed (discussion #40273). -exasol = ["sqlalchemy-exasol>=6.0.0, <8.0"] +# (>=6.0.0) with no dual-compat release. Capped below 6.0.0 for now; bump to +# >=6.0.0,<8.0 in lockstep with Superset's own SQLAlchemy 2.0 core bump +# (discussion #40273), not before. +exasol = ["sqlalchemy-exasol>=2.4.0, <6.0.0"] excel = ["xlrd>=2.0.2, <2.1"] # Async dashboard "Export Data/Images to Excel": uploads the workbook to S3 and # emails a pre-signed link. boto3 is imported lazily by superset.utils.s3, so @@ -193,9 +199,9 @@ fastmcp = [ ] # sqlalchemy-firebird >=2.0.0 unconditionally requires SQLAlchemy 2.0 on # Python >=3.8 (which covers Superset's >=3.11 floor), with no dual-compat -# release. Bumped now that Superset's own SQLAlchemy 2.0 core bump has -# landed (discussion #40273). -firebird = ["sqlalchemy-firebird>=2.2.0"] +# release. Capped below 2.0.0 for now; bump to >=2.2.0 in lockstep with +# Superset's own SQLAlchemy 2.0 core bump (discussion #40273), not before. +firebird = ["sqlalchemy-firebird>=0.8.0, <2.0.0"] firebolt = ["firebolt-sqlalchemy>=1.1.2, <2"] gevent = ["gevent>=26.7.0"] gsheets = ["shillelagh[gsheetsapi]>=1.4.5, <2"] @@ -234,14 +240,15 @@ presto = ["pyhive[presto]>=0.6.5"] trino = ["trino>=0.338.0"] prophet = ["prophet>=1.3.0, <2"] # sqlalchemy-redshift cuts hard from SQLAlchemy 1.4-only (0.8.x) to 2.0-only -# (>=1.0.0) with no dual-compat release. Bumped now that Superset's own -# SQLAlchemy 2.0 core bump has landed (discussion #40273). -redshift = ["sqlalchemy-redshift>=1.0.0"] +# (>=1.0.0) with no dual-compat release; the existing <0.9 ceiling already +# keeps this on the 1.4-only line. Bump to >=1.0.0 in lockstep with +# Superset's own SQLAlchemy 2.0 core bump (discussion #40273), not before. +redshift = ["sqlalchemy-redshift>=0.8.1, <0.9"] # No release of sqlalchemy-risingwave has ever supported both SQLAlchemy 1.4 -# and 2.0 (version numbers don't track SQLAlchemy compat monotonically). -# Bumped to the 2.0-only line now that Superset's own SQLAlchemy 2.0 core -# bump has landed (discussion #40273). -risingwave = ["sqlalchemy-risingwave>=2.0.0"] +# and 2.0 (version numbers don't track SQLAlchemy compat monotonically); pin +# to the newest 1.4-only release for now. Bump to >=2.0.0 in lockstep with +# Superset's own SQLAlchemy 2.0 core bump (discussion #40273), not before. +risingwave = ["sqlalchemy-risingwave>=1.4.1, <3.0.0"] shillelagh = ["shillelagh[all]>=1.4.5, <2"] singlestore = ["sqlalchemy-singlestoredb>=1.2.1, <2"] snowflake = ["snowflake-sqlalchemy>=1.11.0, <2"] diff --git a/pytest.ini b/pytest.ini index f3833da5d53..0a98c8efc05 100644 --- a/pytest.ini +++ b/pytest.ini @@ -23,5 +23,25 @@ python_files = *_test.py test_*.py *_tests.py *viz/utils.py asyncio_mode = auto # `ignore` is effectively equivalent to `-p no:warnings`. +# Always print RemovedIn20Warning when SQLALCHEMY_WARN_20=1. +# Additionally, raise errors for refactored RemovedIn20Warning cases to prevent regression. filterwarnings = ignore + always::sqlalchemy.exc.RemovedIn20Warning + error:Passing a string to Connection.execute\(\) is deprecated:sqlalchemy.exc.RemovedIn20Warning + error:"Query" object is being merged into a Session:sqlalchemy.exc.RemovedIn20Warning + error:"ReportExecutionLog" object is being merged into a Session:sqlalchemy.exc.RemovedIn20Warning + error:"ReportRecipients" object is being merged into a Session:sqlalchemy.exc.RemovedIn20Warning + error:"SavedQuery" object is being merged into a Session:sqlalchemy.exc.RemovedIn20Warning + error:"SqlaTable" object is being merged into a Session:sqlalchemy.exc.RemovedIn20Warning + error:"SqlMetric" object is being merged into a Session:sqlalchemy.exc.RemovedIn20Warning + error:"SSHTunnel" object is being merged into a Session:sqlalchemy.exc.RemovedIn20Warning + error:"TableColumn" object is being merged into a Session:sqlalchemy.exc.RemovedIn20Warning + error:"TaggedObject" object is being merged into a Session:sqlalchemy.exc.RemovedIn20Warning + error:The autoload parameter is deprecated:sqlalchemy.exc.RemovedIn20Warning + error:The connection.execute\(\) method:sqlalchemy.exc.RemovedIn20Warning + error:The current statement is being autocommitted using implicit autocommit:sqlalchemy.exc.RemovedIn20Warning + error:The ``declarative_base\(\)`` function is now available:sqlalchemy.exc.RemovedIn20Warning + error:The Engine.execute\(\) method is considered legacy:sqlalchemy.exc.RemovedIn20Warning + error:The legacy calling style of select\(\) is deprecated:sqlalchemy.exc.RemovedIn20Warning + error:The "whens" argument to case:sqlalchemy.exc.RemovedIn20Warning diff --git a/requirements/base.txt b/requirements/base.txt index 6aa478b2c9a..8b50c990117 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -144,7 +144,7 @@ flask-migrate==4.1.0 # via apache-superset (pyproject.toml) flask-session==0.8.0 # via apache-superset (pyproject.toml) -flask-sqlalchemy==3.1.1 +flask-sqlalchemy==2.5.1 # via # apache-superset (pyproject.toml) # flask-appbuilder @@ -381,7 +381,7 @@ six==1.17.0 # wtforms-json slack-sdk==3.43.0 # via apache-superset (pyproject.toml) -sqlalchemy==2.0.51 +sqlalchemy==1.4.54 # via # apache-superset (pyproject.toml) # alembic @@ -419,7 +419,6 @@ typing-extensions==4.16.0 # pyopenssl # referencing # shillelagh - # sqlalchemy # typing-inspection typing-inspection==0.4.2 # via pydantic diff --git a/requirements/development.txt b/requirements/development.txt index 2b4ff23682f..21047ebecc1 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -306,7 +306,7 @@ flask-session==0.8.0 # via # -c requirements/base-constraint.txt # apache-superset -flask-sqlalchemy==3.1.1 +flask-sqlalchemy==2.5.1 # via # -c requirements/base-constraint.txt # apache-superset @@ -950,7 +950,7 @@ slack-sdk==3.43.0 # apache-superset sniffio==1.3.1 # via anyio -sqlalchemy==2.0.51 +sqlalchemy==1.4.54 # via # -c requirements/base-constraint.txt # alembic @@ -1033,7 +1033,6 @@ typing-extensions==4.16.0 # pyopenssl # referencing # shillelagh - # sqlalchemy # starlette # typing-inspection typing-inspection==0.4.2 diff --git a/superset-core/pyproject.toml b/superset-core/pyproject.toml index 01a406bcce0..74d178fc951 100644 --- a/superset-core/pyproject.toml +++ b/superset-core/pyproject.toml @@ -45,7 +45,7 @@ dependencies = [ "isodate>=0.7.0", "pyarrow>=16.0.0", "pydantic>=2.8.0", - "sqlalchemy>=2.0.0,<2.1", + "sqlalchemy>=1.4.0,<2.0", "sqlalchemy-utils>=0.38.0, <0.43", # expanding lowerbound to work with pydoris "sqlglot>=30.8.0, <31", "typing-extensions>=4.0.0", diff --git a/superset/cli/export_example.py b/superset/cli/export_example.py index cbf4bf6c589..5c14b62dab3 100644 --- a/superset/cli/export_example.py +++ b/superset/cli/export_example.py @@ -155,7 +155,7 @@ def export_example( # noqa: C901 # Find the dashboard if dashboard_id: - dashboard = db.session.get(Dashboard, dashboard_id) + dashboard = db.session.query(Dashboard).get(dashboard_id) elif dashboard_slug: dashboard = db.session.query(Dashboard).filter_by(slug=dashboard_slug).first() else: diff --git a/superset/commands/dataset/duplicate.py b/superset/commands/dataset/duplicate.py index fb1cbb7048e..85cfe290412 100644 --- a/superset/commands/dataset/duplicate.py +++ b/superset/commands/dataset/duplicate.py @@ -65,7 +65,7 @@ class DuplicateDatasetCommand(CreateMixin, BaseCommand): database_id = self._base_model.database_id table_name = self._properties["table_name"] editors = self._properties["editors"] - database = db.session.get(Database, database_id) + database = db.session.query(Database).get(database_id) if not database: raise SupersetErrorException( SupersetError( diff --git a/superset/commands/importers/v1/examples.py b/superset/commands/importers/v1/examples.py index 9cc929f6f14..773fc3c1bcf 100644 --- a/superset/commands/importers/v1/examples.py +++ b/superset/commands/importers/v1/examples.py @@ -69,7 +69,7 @@ def transpile_virtual_dataset_sql(config: dict[str, Any], database_id: int) -> N if not sql: return - database = db.session.get(Database, database_id) + database = db.session.query(Database).get(database_id) if not database: logger.warning("Database %s not found, skipping SQL transpilation", database_id) return diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 88e14873109..4090f990852 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -2528,16 +2528,6 @@ class RowLevelSecurityFilter(Model, AuditMixinNullable): Enum( *[filter_type.value for filter_type in utils.RowLevelSecurityFilterType], name="filter_type_enum", - # No migration has ever created a native "filter_type_enum" type in - # Postgres - the 2020-09-15 migration that added this column only - # ever created a plain VARCHAR. That mismatch was harmless under - # SQLAlchemy 1.4, but SQLAlchemy 2.0's postgresql "insertmanyvalues" - # feature casts every bound parameter to its column type's DDL name - # (`p2::filter_type_enum`) even for a single-row INSERT, which fails - # outright since the type doesn't exist. native_enum=False keeps - # this a plain VARCHAR (with a CHECK constraint) so the type - # actually matches what's really in the database. - native_enum=False, ), ) group_key = Column(String(255), nullable=True) diff --git a/superset/daos/dashboard.py b/superset/daos/dashboard.py index cb04587213a..77b8d89d449 100644 --- a/superset/daos/dashboard.py +++ b/superset/daos/dashboard.py @@ -532,15 +532,6 @@ class DashboardDAO(BaseDAO[Dashboard]): dash.params = original_dash.params cls.set_dash_metadata(dash, metadata, old_to_new_slice_ids) db.session.add(dash) - # Flush so the returned dashboard always has a real, persisted - # identity (dash.id populated) regardless of what the caller does - # next. Without this, whether `dash` ends up with a usable id was an - # accident of whatever query the caller happened to run afterward - # (autoflush would catch it) - the duplicate_slices=True path leaked - # this: it flushes internally per-cloned-slice already, and simple - # test/caller code that queries the DB again incidentally - # autoflushes too, masking that the plain-copy path never did. - db.session.flush() return dash @classmethod diff --git a/superset/daos/dataset.py b/superset/daos/dataset.py index 7b4b6db2ce8..a1df794984d 100644 --- a/superset/daos/dataset.py +++ b/superset/daos/dataset.py @@ -630,7 +630,7 @@ class DatasetDAO(BaseDAO[SqlaTable]): dataset = DatasetDAO.find_by_id(dataset_id) if not dataset: return None - return db.session.get(SqlMetric, metric_id) + return db.session.query(SqlMetric).get(metric_id) @staticmethod def get_table_by_name(database_id: int, table_name: str) -> SqlaTable | None: diff --git a/superset/databases/utils.py b/superset/databases/utils.py index 645248bede6..51666157c92 100644 --- a/superset/databases/utils.py +++ b/superset/databases/utils.py @@ -20,7 +20,6 @@ from __future__ import annotations from typing import Any, TYPE_CHECKING from sqlalchemy.engine.url import make_url, URL -from sqlalchemy.exc import NoSuchTableError from superset.commands.database.exceptions import DatabaseInvalidError from superset.sql.parse import Table @@ -73,42 +72,17 @@ def get_table_metadata(database: Any, table: Table) -> TableMetadataResponse: :return: Dict table metadata ready for API response """ keys = [] - table_missing = False - try: - # get_columns is the table-existence check: SQLAlchemy 2.0's sqlite - # dialect raises NoSuchTableError from reflection for a table that - # doesn't exist - 1.4's sqlite dialect silently returned empty - # results instead, which this API has always relied on to answer - # with an empty-but-200 payload for sqlite specifically (other - # backends' dialects already raised on missing tables pre-2.0, so - # they're unaffected and still surface as the 422 below). Only - # sqlite gets the graceful fallback, matching that pre-existing, - # dialect-driven difference in behavior between backends. Only this - # first call is guarded, so a NoSuchTableError raised later while - # reflecting fks/indexes/comments for a table confirmed to exist - # still propagates instead of being mistaken for a missing table. - columns = database.get_columns(table) - except NoSuchTableError: - if database.backend != "sqlite": - raise - table_missing = True - columns = [] - if not table_missing: - primary_key = database.get_pk_constraint(table) - foreign_keys = get_foreign_keys_metadata(database, table) - indexes = get_indexes_metadata(database, table) - table_comment = database.get_table_comment(table) - else: - primary_key = {"constrained_columns": None, "name": None} - foreign_keys = [] - indexes = [] - table_comment = None + columns = database.get_columns(table) + primary_key = database.get_pk_constraint(table) if primary_key and primary_key.get("constrained_columns"): primary_key["column_names"] = primary_key.pop("constrained_columns") primary_key["type"] = "pk" keys += [primary_key] + foreign_keys = get_foreign_keys_metadata(database, table) + indexes = get_indexes_metadata(database, table) keys += foreign_keys + indexes payload_columns: list[TableMetadataColumnsResponse] = [] + table_comment = database.get_table_comment(table) for col in columns: dtype = get_col_type(col) payload_columns.append( @@ -128,12 +102,7 @@ def get_table_metadata(database: Any, table: Table) -> TableMetadataResponse: show_cols=True if columns else False, indent=True, cols=columns, - # A missing table has no partitions to look up, and asking - # anyway would just re-trigger the same NoSuchTableError via - # select_star()'s own internal database.get_columns() fallback - # (it re-fetches columns itself whenever `cols` is empty and - # either show_cols or latest_partition is set). - latest_partition=not table_missing, + latest_partition=True, ), "primaryKey": primary_key, "foreignKeys": foreign_keys, diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index ebd85bd43a4..3123ee72715 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -2918,19 +2918,17 @@ class BasicParametersMixin: else: query.update(cls.encryption_disable_parameters) - # SQLAlchemy 2.0 made URL.__str__() hide the password by default - # (it rendered in full under 1.4); render_as_string(hide_password= - # False) is required here since this URI is stored/used to actually - # connect, not just displayed. - return URL.create( - f"{cls.engine}+{cls.default_driver}".rstrip("+"), # type: ignore - username=parameters.get("username"), - password=parameters.get("password"), - host=parameters["host"], - port=parameters["port"], - database=parameters["database"], - query=query, - ).render_as_string(hide_password=False) + return str( + URL.create( + f"{cls.engine}+{cls.default_driver}".rstrip("+"), # type: ignore + username=parameters.get("username"), + password=parameters.get("password"), + host=parameters["host"], + port=parameters["port"], + database=parameters["database"], + query=query, + ) + ) @classmethod def get_parameters_from_uri( # pylint: disable=unused-argument diff --git a/superset/db_engine_specs/clickhouse.py b/superset/db_engine_specs/clickhouse.py index 64b65d4fdcb..5057ebd6370 100644 --- a/superset/db_engine_specs/clickhouse.py +++ b/superset/db_engine_specs/clickhouse.py @@ -487,19 +487,17 @@ class ClickHouseConnectEngineSpec(BasicParametersMixin, ClickHouseEngineSpec): if not url_params.get("database"): url_params["database"] = "__default__" - # SQLAlchemy 2.0 made URL.__str__() hide the password by default - # (it rendered in full under 1.4); render_as_string(hide_password= - # False) is required here since this URI is stored/used to actually - # connect, not just displayed. - return URL.create( - f"{cls.engine}+{cls.default_driver}", - username=url_params.get("username"), - password=url_params.get("password"), - host=url_params.get("host"), - port=url_params.get("port"), - database=url_params.get("database"), - query=url_params.get("query"), - ).render_as_string(hide_password=False) + return str( + URL.create( + f"{cls.engine}+{cls.default_driver}", + username=url_params.get("username"), + password=url_params.get("password"), + host=url_params.get("host"), + port=url_params.get("port"), + database=url_params.get("database"), + query=url_params.get("query"), + ) + ) @classmethod def get_parameters_from_uri( diff --git a/superset/db_engine_specs/couchbase.py b/superset/db_engine_specs/couchbase.py index b5001324fd5..9f4b4b82256 100644 --- a/superset/db_engine_specs/couchbase.py +++ b/superset/db_engine_specs/couchbase.py @@ -176,11 +176,7 @@ class CouchbaseEngineSpec(BasicParametersMixin, BaseEngineSpec): query=query_params, ) print(uri) - # SQLAlchemy 2.0 made URL.__str__() hide the password by default - # (it rendered in full under 1.4); render_as_string(hide_password= - # False) is required here since this URI is stored/used to actually - # connect, not just displayed. - return uri.render_as_string(hide_password=False) + return str(uri) @classmethod def get_parameters_from_uri( diff --git a/superset/db_engine_specs/databend.py b/superset/db_engine_specs/databend.py index 759479e7d84..ecd8e21b486 100644 --- a/superset/db_engine_specs/databend.py +++ b/superset/db_engine_specs/databend.py @@ -282,19 +282,17 @@ class DatabendEngineSpec(BasicParametersMixin, DatabendBaseEngineSpec): else cls.encryption_disable_parameters ) - # SQLAlchemy 2.0 made URL.__str__() hide the password by default - # (it rendered in full under 1.4); render_as_string(hide_password= - # False) is required here since this URI is stored/used to actually - # connect, not just displayed. - return URL.create( - cls.engine, - username=parameters.get("username"), - password=parameters.get("password"), - host=parameters.get("host"), - port=parameters.get("port"), - database=parameters.get("database") or "__default__", - query=query, - ).render_as_string(hide_password=False) + return str( + URL.create( + cls.engine, + username=parameters.get("username"), + password=parameters.get("password"), + host=parameters.get("host"), + port=parameters.get("port"), + database=parameters.get("database") or "__default__", + query=query, + ) + ) @classmethod def _encryption_from_tls_parameters( diff --git a/superset/db_engine_specs/databricks.py b/superset/db_engine_specs/databricks.py index c11f66a36f4..63ad680626e 100644 --- a/superset/db_engine_specs/databricks.py +++ b/superset/db_engine_specs/databricks.py @@ -669,19 +669,17 @@ class DatabricksNativeEngineSpec(DatabricksDynamicBaseEngineSpec): ) query.update(cls.encryption_parameters) - # SQLAlchemy 2.0 made URL.__str__() hide the password by default - # (it rendered in full under 1.4); render_as_string(hide_password= - # False) is required here since this URI is stored/used to actually - # connect, not just displayed. - return URL.create( - f"{cls.engine}+{cls.default_driver}".rstrip("+"), - username="token", - password=parameters.get("access_token"), - host=parameters["host"], - port=parameters["port"], - database=parameters["database"], - query=query, - ).render_as_string(hide_password=False) + return str( + URL.create( + f"{cls.engine}+{cls.default_driver}".rstrip("+"), + username="token", + password=parameters.get("access_token"), + host=parameters["host"], + port=parameters["port"], + database=parameters["database"], + query=query, + ) + ) @classmethod def get_parameters_from_uri( # type: ignore @@ -898,18 +896,16 @@ class DatabricksPythonConnectorEngineSpec(DatabricksDynamicBaseEngineSpec): if parameters.get("encryption"): query.update(cls.encryption_parameters) - # SQLAlchemy 2.0 made URL.__str__() hide the password by default - # (it rendered in full under 1.4); render_as_string(hide_password= - # False) is required here since this URI is stored/used to actually - # connect, not just displayed. - return URL.create( - cls.engine, - username="token", - password=parameters.get("access_token"), - host=parameters["host"], - port=parameters["port"], - query=query, - ).render_as_string(hide_password=False) + return str( + URL.create( + cls.engine, + username="token", + password=parameters.get("access_token"), + host=parameters["host"], + port=parameters["port"], + query=query, + ) + ) @classmethod def get_parameters_from_uri( # type: ignore diff --git a/superset/db_engine_specs/duckdb.py b/superset/db_engine_specs/duckdb.py index 87e02c37761..415c38fc533 100644 --- a/superset/db_engine_specs/duckdb.py +++ b/superset/db_engine_specs/duckdb.py @@ -125,11 +125,7 @@ class DuckDBParametersMixin: ): return MotherDuckEngineSpec.build_sqlalchemy_uri(parameters) - # SQLAlchemy 2.0 made URL a strict NamedTuple - the raw URL(...) - # constructor now requires username/password/host/port to be passed - # explicitly (they used to default to None). URL.create() keeps - # those optional, matching the pre-2.0 URL(...) behavior used here. - return str(URL.create(drivername=cls.engine, database=database, query=query)) + return str(URL(drivername=cls.engine, database=database, query=query)) @classmethod def get_parameters_from_uri( # pylint: disable=unused-argument @@ -414,14 +410,8 @@ class MotherDuckEngineSpec(DuckDBEngineSpec): f"Need MotherDuck token to connect to database '{database}'." ) - # SQLAlchemy 2.0 made URL a strict NamedTuple - the raw URL(...) - # constructor now requires username/password/host/port to be passed - # explicitly (they used to default to None). URL.create() keeps - # those optional, matching the pre-2.0 URL(...) behavior used here. return str( - URL.create( - drivername=DuckDBEngineSpec.engine, database=database, query=query - ) + URL(drivername=DuckDBEngineSpec.engine, database=database, query=query) ) @classmethod diff --git a/superset/db_engine_specs/snowflake.py b/superset/db_engine_specs/snowflake.py index 0687464ac22..683d892de88 100644 --- a/superset/db_engine_specs/snowflake.py +++ b/superset/db_engine_specs/snowflake.py @@ -356,21 +356,19 @@ class SnowflakeEngineSpec(PostgresBaseEngineSpec): dict[str, Any] ] = None, ) -> str: - # SQLAlchemy 2.0 made URL.__str__() hide the password by default - # (it rendered in full under 1.4); render_as_string(hide_password= - # False) is required here since this URI is stored/used to actually - # connect, not just displayed. - return URL.create( - "snowflake", - username=parameters.get("username"), - password=parameters.get("password"), - host=parameters.get("account"), - database=parameters.get("database"), - query={ - "role": parameters.get("role"), - "warehouse": parameters.get("warehouse"), - }, - ).render_as_string(hide_password=False) + return str( + URL.create( + "snowflake", + username=parameters.get("username"), + password=parameters.get("password"), + host=parameters.get("account"), + database=parameters.get("database"), + query={ + "role": parameters.get("role"), + "warehouse": parameters.get("warehouse"), + }, + ) + ) @classmethod def get_parameters_from_uri( diff --git a/superset/extensions/__init__.py b/superset/extensions/__init__.py index b5035b73c24..35df0900a9b 100644 --- a/superset/extensions/__init__.py +++ b/superset/extensions/__init__.py @@ -154,19 +154,7 @@ async_query_manager: AsyncQueryManager = LocalProxy( cache_manager = CacheManager() celery_app = celery.Celery() csrf = CSRFProtect() - -# Flask-SQLAlchemy 3.x scopes db.session by the identity of the current Flask -# app-context object (id(app_ctx)) rather than by thread/greenlet identity like -# 2.x did. Superset's codebase (and its test fixtures) widely assumes a single -# shared session per thread across nested `app.app_context()` blocks, often -# relying on that implicit sharing instead of an explicit commit. Restoring the -# 2.x scopefunc here keeps that assumption valid under FSA 3.x. -try: - from greenlet import getcurrent as _session_scopefunc -except ImportError: - from threading import get_ident as _session_scopefunc - -db = get_sqla_class()(session_options={"scopefunc": _session_scopefunc}) +db = get_sqla_class()() # make_versioned() MUST be called immediately after db is constructed and before # any versioned model class is defined. Continuum patches the SQLAlchemy diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 434cc912af7..f2f089d37b2 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -26,16 +26,7 @@ from typing import Any, Callable, TYPE_CHECKING import wtforms_json from colorama import Fore, Style from deprecation import deprecated -from flask import ( - abort, - current_app, - Flask, - has_app_context, - redirect, - request, - session, - url_for, -) +from flask import abort, current_app, Flask, redirect, request, session, url_for from flask_appbuilder import expose, IndexView from flask_appbuilder.api import safe from flask_appbuilder.utils.base import get_safe_redirect @@ -156,16 +147,8 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods # pylint: disable=too-few-public-methods abstract = True - # Grab each call into the task and set up an app context, unless - # one is already active on this thread (e.g. Celery eager mode - # invoked from within an existing request/test context) - Flask- - # SQLAlchemy 3.x scopes db.session by the active app context's - # object identity rather than by thread, so pushing a redundant - # nested context here would silently hand the task a second, - # blind session unable to see the caller's uncommitted work. + # Grab each call into the task and set up an app context def __call__(self, *args: Any, **kwargs: Any) -> Any: - if has_app_context(): - return task_base.__call__(self, *args, **kwargs) with superset_app.app_context(): return task_base.__call__(self, *args, **kwargs) diff --git a/superset/mcp_service/chart/preview_utils.py b/superset/mcp_service/chart/preview_utils.py index a9e99e814e4..961569421bc 100644 --- a/superset/mcp_service/chart/preview_utils.py +++ b/superset/mcp_service/chart/preview_utils.py @@ -69,7 +69,7 @@ def generate_preview_from_form_data( from superset.connectors.sqla.models import SqlaTable from superset.extensions import db - dataset = db.session.get(SqlaTable, dataset_id) + dataset = db.session.query(SqlaTable).get(dataset_id) if not dataset: return ChartError( error=f"Dataset {dataset_id} not found", error_type="DatasetNotFound" diff --git a/superset/migrations/versions/2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py b/superset/migrations/versions/2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py index 14cc7f2ce2b..9fbf0284eea 100644 --- a/superset/migrations/versions/2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py +++ b/superset/migrations/versions/2018-07-26_11-10_c82ee8a39623_add_implicit_tags.py @@ -30,7 +30,7 @@ from datetime import datetime # noqa: E402 from alembic import op # noqa: E402 from sqlalchemy import Column, DateTime, Enum, ForeignKey, Integer, String # noqa: E402 -from sqlalchemy.orm import declarative_base, declared_attr, Mapped # noqa: E402 +from sqlalchemy.orm import declarative_base, declared_attr # noqa: E402 from superset.tags.models import ObjectType, TagType # noqa: E402 from superset.utils.core import get_user_id # noqa: E402 @@ -55,7 +55,7 @@ class AuditMixinNullable: ) @declared_attr - def created_by_fk(self) -> Mapped[int | None]: + def created_by_fk(self) -> Column: return Column( Integer, ForeignKey("ab_user.id"), @@ -64,7 +64,7 @@ class AuditMixinNullable: ) @declared_attr - def changed_by_fk(self) -> Mapped[int | None]: + def changed_by_fk(self) -> Column: return Column( Integer, ForeignKey("ab_user.id"), diff --git a/superset/migrations/versions/2020-01-08_01-17_e96dbf2cfef0_datasource_cluster_fk.py b/superset/migrations/versions/2020-01-08_01-17_e96dbf2cfef0_datasource_cluster_fk.py index 565e66b6836..770f4ea7bde 100644 --- a/superset/migrations/versions/2020-01-08_01-17_e96dbf2cfef0_datasource_cluster_fk.py +++ b/superset/migrations/versions/2020-01-08_01-17_e96dbf2cfef0_datasource_cluster_fk.py @@ -44,7 +44,7 @@ def upgrade(): batch_op.add_column(sa.Column("cluster_id", sa.Integer())) # Update cluster_id values - metadata = sa.MetaData() + metadata = sa.MetaData(bind=bind) datasources = sa.Table("datasources", metadata, autoload_with=bind) clusters = sa.Table("clusters", metadata, autoload_with=bind) @@ -86,7 +86,7 @@ def downgrade(): batch_op.add_column(sa.Column("cluster_name", sa.String(250))) # Update cluster_name values - metadata = sa.MetaData() + metadata = sa.MetaData(bind=bind) datasources = sa.Table("datasources", metadata, autoload_with=bind) clusters = sa.Table("clusters", metadata, autoload_with=bind) diff --git a/superset/migrations/versions/2020-04-24_10-46_e557699a813e_add_tables_relation_to_row_level_.py b/superset/migrations/versions/2020-04-24_10-46_e557699a813e_add_tables_relation_to_row_level_.py index 9567c2629cc..42156e177ef 100644 --- a/superset/migrations/versions/2020-04-24_10-46_e557699a813e_add_tables_relation_to_row_level_.py +++ b/superset/migrations/versions/2020-04-24_10-46_e557699a813e_add_tables_relation_to_row_level_.py @@ -35,7 +35,7 @@ down_revision = "743a117f0d98" def upgrade(): bind = op.get_bind() - metadata = sa.MetaData() + metadata = sa.MetaData(bind=bind) insp = sa.engine.reflection.Inspector.from_engine(bind) rls_filter_tables = create_table( @@ -53,7 +53,7 @@ def upgrade(): for row in bind.execute(filter_ids): move_table_id = rls_filter_tables.insert().values( - rls_filter_id=row._mapping["id"], table_id=row._mapping["table_id"] + rls_filter_id=row["id"], table_id=row["table_id"] ) bind.execute(move_table_id) @@ -68,7 +68,7 @@ def upgrade(): def downgrade(): bind = op.get_bind() - metadata = sa.MetaData() + metadata = sa.MetaData(bind=bind) op.add_column( "row_level_security_filters", diff --git a/superset/migrations/versions/2020-09-15_18-22_e5ef6828ac4e_add_rls_filter_type_and_grouping_key.py b/superset/migrations/versions/2020-09-15_18-22_e5ef6828ac4e_add_rls_filter_type_and_grouping_key.py index ac67f7c3ac2..02c3be3ce0c 100644 --- a/superset/migrations/versions/2020-09-15_18-22_e5ef6828ac4e_add_rls_filter_type_and_grouping_key.py +++ b/superset/migrations/versions/2020-09-15_18-22_e5ef6828ac4e_add_rls_filter_type_and_grouping_key.py @@ -43,7 +43,7 @@ def upgrade(): ) bind = op.get_bind() - metadata = sa.MetaData() + metadata = sa.MetaData(bind=bind) filters = sa.Table("row_level_security_filters", metadata, autoload_with=bind) statement = filters.update().values( filter_type=utils.RowLevelSecurityFilterType.REGULAR.value diff --git a/superset/migrations/versions/2020-09-24_12-04_3fbbc6e8d654_fix_data_access_permissions_for_virtual_.py b/superset/migrations/versions/2020-09-24_12-04_3fbbc6e8d654_fix_data_access_permissions_for_virtual_.py index 8a72975efc5..5eabe3f1618 100644 --- a/superset/migrations/versions/2020-09-24_12-04_3fbbc6e8d654_fix_data_access_permissions_for_virtual_.py +++ b/superset/migrations/versions/2020-09-24_12-04_3fbbc6e8d654_fix_data_access_permissions_for_virtual_.py @@ -168,7 +168,7 @@ def upgrade(): # noqa: C901 match_ds_id = re.match(r"\[None\]\.\[.*\]\(id:(\d+)\)", faulty_view_menu.name) if match_ds_id: dataset_id = int(match_ds_id.group(1)) - dataset = session.get(SqlaTable, dataset_id) + dataset = session.query(SqlaTable).get(dataset_id) if dataset: try: new_view_menu = dataset.get_perm() diff --git a/superset/migrations/versions/2020-09-28_17-57_b56500de1855_add_uuid_column_to_import_mixin.py b/superset/migrations/versions/2020-09-28_17-57_b56500de1855_add_uuid_column_to_import_mixin.py index 7ab8a90cdb1..d417c134efc 100644 --- a/superset/migrations/versions/2020-09-28_17-57_b56500de1855_add_uuid_column_to_import_mixin.py +++ b/superset/migrations/versions/2020-09-28_17-57_b56500de1855_add_uuid_column_to_import_mixin.py @@ -140,11 +140,10 @@ def upgrade(): batch_op.create_unique_constraint(f"uq_{table_name}_uuid", ["uuid"]) # add UUID to Dashboard.position_json - slices_model = models["slices"] slice_uuid_map = { slc.id: slc.uuid - for slc in session.query(slices_model) - .options(load_only(slices_model.id, slices_model.uuid)) + for slc in session.query(models["slices"]) + .options(load_only("id", "uuid")) .all() } update_dashboards(session, slice_uuid_map) diff --git a/superset/migrations/versions/2021-02-04_09-34_070c043f2fdb_add_granularity_to_charts_where_missing.py b/superset/migrations/versions/2021-02-04_09-34_070c043f2fdb_add_granularity_to_charts_where_missing.py index 6c8fc02e10c..7e3d7895ef7 100644 --- a/superset/migrations/versions/2021-02-04_09-34_070c043f2fdb_add_granularity_to_charts_where_missing.py +++ b/superset/migrations/versions/2021-02-04_09-34_070c043f2fdb_add_granularity_to_charts_where_missing.py @@ -92,7 +92,7 @@ def upgrade(): if "granularity" in params or "granularity_sqla" in params: continue - table = session.get(SqlaTable, slc.datasource_id) + table = session.query(SqlaTable).get(slc.datasource_id) if not table: continue diff --git a/superset/migrations/versions/2021-02-18_09-13_c501b7c653a3_add_missing_uuid_column.py b/superset/migrations/versions/2021-02-18_09-13_c501b7c653a3_add_missing_uuid_column.py index aa68490812d..93e645e7e1b 100644 --- a/superset/migrations/versions/2021-02-18_09-13_c501b7c653a3_add_missing_uuid_column.py +++ b/superset/migrations/versions/2021-02-18_09-13_c501b7c653a3_add_missing_uuid_column.py @@ -87,11 +87,10 @@ def upgrade(): # add UUID to Dashboard.position_json; this function is idempotent # so we can call it for all objects - slices_model = models["slices"] slice_uuid_map = { slc.id: slc.uuid - for slc in session.query(slices_model) - .options(load_only(slices_model.id, slices_model.uuid)) + for slc in session.query(models["slices"]) + .options(load_only("id", "uuid")) .all() } update_dashboards(session, slice_uuid_map) diff --git a/superset/migrations/versions/2022-04-01_14-38_a9422eeaae74_new_dataset_models_take_2.py b/superset/migrations/versions/2022-04-01_14-38_a9422eeaae74_new_dataset_models_take_2.py index 93962991b75..f93b3acf1cf 100644 --- a/superset/migrations/versions/2022-04-01_14-38_a9422eeaae74_new_dataset_models_take_2.py +++ b/superset/migrations/versions/2022-04-01_14-38_a9422eeaae74_new_dataset_models_take_2.py @@ -35,7 +35,6 @@ from sqlalchemy.orm import ( backref, declarative_base, declared_attr, - Mapped, relationship, Session, ) @@ -192,7 +191,7 @@ class SqlaTable(AuxiliaryColumnsMixin, Base): id = sa.Column(sa.Integer, primary_key=True) extra = sa.Column(sa.Text) database_id = sa.Column(sa.Integer, sa.ForeignKey("dbs.id"), nullable=False) - database: Mapped[Database] = relationship( + database: Database = relationship( "Database", backref=backref("tables", cascade="all, delete-orphan"), foreign_keys=[database_id], @@ -275,7 +274,7 @@ class NewTable(AuxiliaryColumnsMixin, Base): name = sa.Column(sa.Text) external_url = sa.Column(sa.Text, nullable=True) extra_json = sa.Column(MediumText(), default="{}") - database: Mapped[Database] = relationship( + database: Database = relationship( "Database", backref=backref("new_tables", cascade="all, delete-orphan"), foreign_keys=[database_id], diff --git a/superset/models/core.py b/superset/models/core.py index 7a4fad81d88..3158e690b95 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -32,6 +32,7 @@ from datetime import datetime from functools import lru_cache from inspect import signature from typing import Any, Callable, cast, Optional, TYPE_CHECKING +from urllib.parse import quote import numpy import pandas as pd @@ -505,14 +506,7 @@ class Database(CoreDatabase, AuditMixinNullable, ImportExportMixin): # pylint: # do not over-write the password with the password mask self.password = conn.password conn = conn.set(password=PASSWORD_MASK if conn.password else None) - # Store the literal PASSWORD_MASK sentinel (not the real secret - - # that already went to self.password above), so later code that - # compares conn.password against PASSWORD_MASK to detect an - # unchanged password keeps working. SQLAlchemy 2.0 changed - # str(URL) to substitute its own "***" for any password rather - # than rendering the value verbatim (str(conn) under 1.4), so - # render_as_string(hide_password=False) is required here. - self.sqlalchemy_uri = conn.render_as_string(hide_password=False) + self.sqlalchemy_uri = str(conn) # hides the password def get_effective_user(self, object_url: URL) -> str | None: """ @@ -715,14 +709,7 @@ class Database(CoreDatabase, AuditMixinNullable, ImportExportMixin): # pylint: if cacheable and self.id is not None: cache_key = ( self.id, - # SQLAlchemy 2.0 changed str(URL) to always substitute - # "***" for the password rather than rendering the real - # value (str(url) under 1.4). Using it here would make - # the cache key blind to password rotation - the module - # comment above depends on the key changing when the - # password does, so render_as_string(hide_password=False) - # is required to preserve that behavior. - sqlalchemy_url.render_as_string(hide_password=False), + str(sqlalchemy_url), repr(sorted(engine_kwargs.items())), ) with _ENGINE_CACHE_LOCK: @@ -1392,15 +1379,13 @@ class Database(CoreDatabase, AuditMixinNullable, ImportExportMixin): # pylint: else: raw_password = self.password - # URL.render_as_string() percent-encodes the password itself, so pass - # the raw, un-encoded password straight through. Pre-encoding it here - # (as this used to do) is safe under SQLAlchemy 1.4, whose - # render_as_string() treats URL.password as literal, but SQLAlchemy - # 2.0 always encodes on render - double-encoding a pre-escaped - # password (e.g. "%" -> "%25") turns it into "%2525" on write, which - # then decodes back to the wrong value ("%25" instead of "%") on the - # next reparse. - conn = conn.set(password=raw_password) + # Encode the password such that special characters + # are preserved when rendering to string and reparsing the URL. + if raw_password is not None: + encoded_password = quote(raw_password, safe="") + conn = conn.set(password=encoded_password) + else: + conn = conn.set(password=None) # render_as_string preserves the URL encoding of special # characters in passwords diff --git a/superset/models/sql_types/presto_sql_types.py b/superset/models/sql_types/presto_sql_types.py index 245183c67b0..bf770553479 100644 --- a/superset/models/sql_types/presto_sql_types.py +++ b/superset/models/sql_types/presto_sql_types.py @@ -16,7 +16,7 @@ # under the License. # pylint: disable=abstract-method -from typing import Any, Callable, Optional +from typing import Any, Optional from sqlalchemy.engine.interfaces import Dialect from sqlalchemy.sql.sqltypes import DATE, Integer, TIMESTAMP @@ -113,23 +113,6 @@ class TimeStamp(TypeDecorator): """ return f"TIMESTAMP '{value}'" - def literal_processor(self, dialect: Dialect) -> Callable[[str], str]: - """ - Used when compiling with literal_binds=True (e.g. where_latest_partition). - - TypeDecorator's standard composition model (overriding - process_literal_param/process_bind_param) expects those hooks to - return a plain value that the impl TIMESTAMP type's own processor - then converts to SQL text - but process_bind_param here already - returns the final literal text ("TIMESTAMP '...'"). Composing that - through TIMESTAMP's literal_processor breaks under SQLAlchemy 2.0 - (it expects a real datetime and raises CompileError on the string). - Overriding literal_processor directly - against the base class's own - advice - is the only way to keep this class's "process_bind_param - already produces final SQL text" design working. - """ - return lambda value: self.process_bind_param(value, dialect) - class Date(TypeDecorator): """ @@ -146,12 +129,3 @@ class Date(TypeDecorator): as Presto does not support automatic casting. """ return f"DATE '{value}'" - - def literal_processor(self, dialect: Dialect) -> Callable[[str], str]: - """ - Used when compiling with literal_binds=True (e.g. where_latest_partition). - - See TimeStamp.literal_processor above for why this override is - needed under SQLAlchemy 2.0. - """ - return lambda value: self.process_bind_param(value, dialect) diff --git a/superset/security/manager.py b/superset/security/manager.py index a7a23f6c47a..dbd3935997d 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -3091,7 +3091,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods logger.warning( "Dataset has no database will retry with database_id to set permission" ) - database = self.session.get(Database, target.database_id) + database = self.session.query(Database).get(target.database_id) dataset_perm = self.get_dataset_perm( target.id, target.table_name, database.database_name ) diff --git a/superset/tasks/context.py b/superset/tasks/context.py index 07f95589acf..4120edb5524 100644 --- a/superset/tasks/context.py +++ b/superset/tasks/context.py @@ -20,10 +20,9 @@ import logging import threading import time import traceback -from contextlib import nullcontext from typing import Any, Callable, cast, TYPE_CHECKING, TypeVar -from flask import current_app, has_app_context +from flask import current_app from superset_core.tasks.types import ( TaskContext as CoreTaskContext, TaskProperties, @@ -257,7 +256,7 @@ class TaskContext(CoreTaskContext): if self._has_pending_updates: # Need app context for DB operations in timer thread - if self._app and not has_app_context(): + if self._app: with self._app.app_context(): self._write_to_db() else: @@ -476,8 +475,7 @@ class TaskContext(CoreTaskContext): ) if self._app: - ctx = self._app.app_context() if not has_app_context() else nullcontext() - with ctx: + with self._app.app_context(): # Check if task already has an error (preserve original context) task = self._task original_error = task.properties_dict.get("error_message") @@ -552,8 +550,7 @@ class TaskContext(CoreTaskContext): ) return - ctx = self._app.app_context() if not has_app_context() else nullcontext() - with ctx: + with self._app.app_context(): from superset.commands.tasks.update import UpdateTaskCommand task = self._task @@ -640,8 +637,7 @@ class TaskContext(CoreTaskContext): # If aborting/aborted but handlers haven't run yet, run them now # (This catches the case where task ended before listener detected abort) if self._app: - ctx = self._app.app_context() if not has_app_context() else nullcontext() - with ctx: + with self._app.app_context(): task = self._task if task.status in ABORT_STATES and not self._abort_detected: self._trigger_abort_handlers() diff --git a/superset/tasks/manager.py b/superset/tasks/manager.py index 6778ea03181..08dcdc26b99 100644 --- a/superset/tasks/manager.py +++ b/superset/tasks/manager.py @@ -25,7 +25,6 @@ from typing import Any, Callable, TYPE_CHECKING from uuid import UUID import redis -from flask import has_app_context from superset_core.tasks.types import TaskProperties, TaskScope from superset.async_events.cache_backend import ( @@ -259,7 +258,7 @@ class TaskManager: return remaining if remaining > 0 else 0 def get_task() -> "Task | None": - if app and not has_app_context(): + if app: with app.app_context(): return TaskDAO.find_one_or_none(uuid=task_uuid) return TaskDAO.find_one_or_none(uuid=task_uuid) @@ -462,7 +461,7 @@ class TaskManager: :param callback: Function to invoke :param app: Flask app for context, or None """ - if app and not has_app_context(): + if app: with app.app_context(): callback() else: @@ -660,7 +659,7 @@ class TaskManager: def check_database() -> bool: # Need app context for database access - if app and not has_app_context(): + if app: with app.app_context(): return cls._check_abort_status(task_uuid) else: diff --git a/superset/utils/core.py b/superset/utils/core.py index 1c3b92ebd65..e55189f9468 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -816,7 +816,6 @@ def pessimistic_connection_handling(some_engine: Engine) -> None: # the SELECT of a scalar value without a table is # appropriately formatted for the backend connection.scalar(select(1)) - connection.rollback() # pylint: disable=consider-using-transaction except exc.DBAPIError as err: # catch SQLAlchemy's DBAPIError, which is a wrapper # for the DBAPI's exception. It includes a .connection_invalidated @@ -829,7 +828,6 @@ def pessimistic_connection_handling(some_engine: Engine) -> None: # here also causes the whole connection pool to be invalidated # so that all stale connections are discarded. connection.scalar(select(1)) - connection.rollback() # pylint: disable=consider-using-transaction else: raise finally: diff --git a/superset/views/core.py b/superset/views/core.py index 711f5f5525a..029513ce790 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -462,11 +462,6 @@ class Superset(BaseSupersetView): g.user.id if g.user else None ), ) - # SQLAlchemy 2.0 removes the legacy cascade_backrefs behavior, so - # appending `slc` (persistent) to this new, transient dash.slices - # below no longer implicitly adds `dash` to the session via the - # Slice.dashboards backref - it must be added explicitly. - db.session.add(dash) if dash and slc not in dash.slices: dash.slices.append(slc) diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index c32272b0e63..803de3a033b 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -165,11 +165,6 @@ class TestChartApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCase): crontab="* * * * *", chart=chart, ) - # SQLAlchemy 2.0 removes the legacy cascade_backrefs behavior, so - # assigning `chart=chart` on a transient ReportSchedule no longer - # implicitly adds it to the session via the Slice.report_schedules - # backref - it must be added explicitly. - db.session.add(report_schedule) db.session.commit() yield chart diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 52857d5f572..54b456e0801 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -184,11 +184,6 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa crontab="* * * * *", dashboard=dashboard, ) - # SQLAlchemy 2.0 removes the legacy cascade_backrefs behavior, so - # assigning `dashboard=dashboard` on a transient ReportSchedule no - # longer implicitly adds it to the session via the - # Dashboard.report_schedules backref - it must be added explicitly. - db.session.add(report_schedule) db.session.commit() yield dashboard diff --git a/tests/integration_tests/dashboards/soft_delete_tests.py b/tests/integration_tests/dashboards/soft_delete_tests.py index 9174a6d59f0..1509ceca975 100644 --- a/tests/integration_tests/dashboards/soft_delete_tests.py +++ b/tests/integration_tests/dashboards/soft_delete_tests.py @@ -353,10 +353,9 @@ class TestDashboardRestore(SupersetTestCase): functional key parts the partial index needs in 8.0.13 (8.0.0–8.0.12 reject it). """ - bind = db.session.get_bind() - dialect = bind.dialect.name - is_mariadb = getattr(bind.dialect, "is_mariadb", False) - server_version = bind.dialect.server_version_info or () + dialect = db.session.bind.dialect.name + is_mariadb = getattr(db.session.bind.dialect, "is_mariadb", False) + server_version = db.session.bind.dialect.server_version_info or () partial_index_supported = dialect == "postgresql" or ( dialect == "mysql" and not is_mariadb and server_version >= (8, 0, 13) ) diff --git a/tests/integration_tests/datasource_tests.py b/tests/integration_tests/datasource_tests.py index d75e517001a..407bbfddb76 100644 --- a/tests/integration_tests/datasource_tests.py +++ b/tests/integration_tests/datasource_tests.py @@ -107,6 +107,9 @@ def create_and_cleanup_table(table=None): class TestDatasource(SupersetTestCase): + def setUp(self): + db.session.begin(subtransactions=True) + def tearDown(self): db.session.rollback() super().tearDown() diff --git a/tests/integration_tests/db_engine_specs/hive_tests.py b/tests/integration_tests/db_engine_specs/hive_tests.py index 6b65a1d52c0..df6db6723af 100644 --- a/tests/integration_tests/db_engine_specs/hive_tests.py +++ b/tests/integration_tests/db_engine_specs/hive_tests.py @@ -357,9 +357,7 @@ def test_where_latest_partition(mock_method): columns, ) query_result = str(result.compile(compile_kwargs={"literal_binds": True})) - # SQLAlchemy 2.0 changed how select() with no columns renders - a single - # trailing space before the newline instead of two under 1.4. - assert "SELECT \nWHERE ds = '01-01-19' AND hour = 1" == query_result + assert "SELECT \nWHERE ds = '01-01-19' AND hour = 1" == query_result @mock.patch("superset.db_engine_specs.presto.PrestoEngineSpec.latest_partition") diff --git a/tests/integration_tests/db_engine_specs/presto_tests.py b/tests/integration_tests/db_engine_specs/presto_tests.py index 66473246caf..3f8d8b51c5a 100644 --- a/tests/integration_tests/db_engine_specs/presto_tests.py +++ b/tests/integration_tests/db_engine_specs/presto_tests.py @@ -610,9 +610,7 @@ class TestPrestoDbEngineSpec(SupersetTestCase): columns, ) query_result = str(result.compile(compile_kwargs={"literal_binds": True})) - # SQLAlchemy 2.0 changed how select() with no columns renders - a - # single trailing space before the newline instead of two under 1.4. - assert "SELECT \nWHERE ds = '01-01-19' AND hour = 1" == query_result + assert "SELECT \nWHERE ds = '01-01-19' AND hour = 1" == query_result def test_query_cost_formatter(self): raw_cost = [ diff --git a/tests/integration_tests/model_tests.py b/tests/integration_tests/model_tests.py index 5d25c9babcd..082395d4016 100644 --- a/tests/integration_tests/model_tests.py +++ b/tests/integration_tests/model_tests.py @@ -211,14 +211,7 @@ class TestDatabaseModel(SupersetTestCase): model._get_sqla_engine() call_args = mocked_create_engine.call_args - # SQLAlchemy 2.0 changed URL.__str__() to hide the password by - # default (it used to render it in full under 1.4); use - # render_as_string(hide_password=False) to compare the real, - # unmasked URL the engine was actually created with. - assert ( - call_args[0][0].render_as_string(hide_password=False) - == "mysql://user:password@localhost" - ) + assert str(call_args[0][0]) == "mysql://user:password@localhost" assert call_args[1]["connect_args"]["local_infile"] == 0 model = Database( @@ -228,10 +221,7 @@ class TestDatabaseModel(SupersetTestCase): model._get_sqla_engine() call_args = mocked_create_engine.call_args - assert ( - call_args[0][0].render_as_string(hide_password=False) - == "mysql+mysqlconnector://user:password@localhost" - ) + assert str(call_args[0][0]) == "mysql+mysqlconnector://user:password@localhost" assert call_args[1]["connect_args"]["allow_local_infile"] == 0 @mock.patch("superset.models.core.create_engine") @@ -259,12 +249,8 @@ class TestDatabaseModel(SupersetTestCase): model._get_sqla_engine() call_args = mocked_create_engine.call_args - # SQLAlchemy 2.0 changed URL.__str__() to hide the password by - # default (it used to render it in full under 1.4); use - # render_as_string(hide_password=False) to compare the real, - # unmasked URL the engine was actually created with. assert ( - call_args[0][0].render_as_string(hide_password=False) + str(call_args[0][0]) == "trino://original_user:original_user_password@localhost/" ) assert call_args[1]["connect_args"]["user"] == "gamma" diff --git a/tests/integration_tests/superset_test_config.py b/tests/integration_tests/superset_test_config.py index eb6567a7478..0ba4082a079 100644 --- a/tests/integration_tests/superset_test_config.py +++ b/tests/integration_tests/superset_test_config.py @@ -21,7 +21,6 @@ from copy import copy from datetime import timedelta from sqlalchemy.engine import make_url -from sqlalchemy.pool import NullPool from superset.config import * # noqa: F403 from superset.config import DATA_DIR @@ -59,23 +58,6 @@ if make_url(SQLALCHEMY_DATABASE_URI).get_backend_name() == "sqlite": "SQLite Database support for metadata databases will be " "removed in a future version of Superset." ) - # SQLAlchemy 2.0 changed the default poolclass for file-based SQLite - # engines from NullPool to QueuePool (SQLAlchemy 1.4 always created a - # fresh low-level connection per checkout for sqlite files, so a pooled - # connection was never handed to a thread other than the one that opened - # it). QueuePool reuses connections across checkouts, including checkouts - # from background threads (e.g. the GTF task framework's deferred-flush - # timer in superset/tasks/context.py). Combined with our - # `?check_same_thread=true` test URIs, a connection opened on the main - # thread can now be handed to a timer thread, which pysqlite rejects with - # "SQLite objects created in a thread can only be used in that same - # thread." Pin poolclass back to NullPool to restore the 1.4 behavior for - # the test suite specifically; Superset's non-test default config uses - # `check_same_thread=false`, which is unaffected by this pool reuse. - SQLALCHEMY_ENGINE_OPTIONS = { # noqa: F405 - **SQLALCHEMY_ENGINE_OPTIONS, # noqa: F405 - "poolclass": NullPool, # noqa: F405 - } # Speeding up the tests.integration_tests. PRESTO_POLL_INTERVAL = 0.1 diff --git a/tests/integration_tests/versioning/id_reuse_tests.py b/tests/integration_tests/versioning/id_reuse_tests.py index a3f3bda5584..20b4223bcfc 100644 --- a/tests/integration_tests/versioning/id_reuse_tests.py +++ b/tests/integration_tests/versioning/id_reuse_tests.py @@ -251,7 +251,7 @@ def test_shadow_rows_for_both_entities_share_the_id( assert successor.uuid in uuids, ( f"successor's shadow rows missing under the recycled id; found {uuids}" ) - assert sa.inspect(db.session.get_bind()).has_table("slices_version") + assert sa.inspect(db.session.bind).has_table("slices_version") def test_list_versions_excludes_the_predecessors_rows( diff --git a/tests/unit_tests/commands/dataset/test_duplicate.py b/tests/unit_tests/commands/dataset/test_duplicate.py index aeb6efc176b..f03b921c949 100644 --- a/tests/unit_tests/commands/dataset/test_duplicate.py +++ b/tests/unit_tests/commands/dataset/test_duplicate.py @@ -198,8 +198,10 @@ def test_duplicate_dataset_success() -> None: ), patch("superset.commands.dataset.duplicate.security_manager.raise_for_access"), ): - with patch("superset.commands.dataset.duplicate.db.session.get") as mock_get: - mock_get.return_value = mock_database + with patch( + "superset.commands.dataset.duplicate.db.session.query" + ) as mock_query: + mock_query.return_value.get.return_value = mock_database with patch( "superset.commands.dataset.duplicate.DatasetDAO.validate_uniqueness", return_value=True, @@ -367,8 +369,10 @@ def test_duplicate_dataset_catalog_preserved() -> None: ), patch("superset.commands.dataset.duplicate.security_manager.raise_for_access"), ): - with patch("superset.commands.dataset.duplicate.db.session.get") as mock_get: - mock_get.return_value = mock_database + with patch( + "superset.commands.dataset.duplicate.db.session.query" + ) as mock_query: + mock_query.return_value.get.return_value = mock_database with patch( "superset.commands.dataset.duplicate.DatasetDAO.validate_uniqueness", return_value=True, @@ -494,8 +498,10 @@ def test_duplicate_dataset_with_columns_and_metrics() -> None: ), patch("superset.commands.dataset.duplicate.security_manager.raise_for_access"), ): - with patch("superset.commands.dataset.duplicate.db.session.get") as mock_get: - mock_get.return_value = mock_database + with patch( + "superset.commands.dataset.duplicate.db.session.query" + ) as mock_query: + mock_query.return_value.get.return_value = mock_database with patch( "superset.commands.dataset.duplicate.DatasetDAO.validate_uniqueness", return_value=True, diff --git a/tests/unit_tests/commands/importers/v1/examples_test.py b/tests/unit_tests/commands/importers/v1/examples_test.py index 71bd1dcac1c..be1134a7ad8 100644 --- a/tests/unit_tests/commands/importers/v1/examples_test.py +++ b/tests/unit_tests/commands/importers/v1/examples_test.py @@ -39,7 +39,7 @@ def test_transpile_virtual_dataset_sql_empty_sql(): @patch("superset.commands.importers.v1.examples.db") def test_transpile_virtual_dataset_sql_database_not_found(mock_db): """Test graceful handling when database is not found.""" - mock_db.session.get.return_value = None + mock_db.session.query.return_value.get.return_value = None config = {"table_name": "my_table", "sql": "SELECT * FROM foo"} original_sql = config["sql"] @@ -56,7 +56,7 @@ def test_transpile_virtual_dataset_sql_success(mock_transpile, mock_db): """Test successful SQL transpilation with source engine.""" mock_database = MagicMock() mock_database.db_engine_spec.engine = "mysql" - mock_db.session.get.return_value = mock_database + mock_db.session.query.return_value.get.return_value = mock_database mock_transpile.return_value = "SELECT * FROM `foo`" @@ -77,7 +77,7 @@ def test_transpile_virtual_dataset_sql_no_source_engine(mock_transpile, mock_db) """Test transpilation when source_db_engine is not specified (legacy).""" mock_database = MagicMock() mock_database.db_engine_spec.engine = "mysql" - mock_db.session.get.return_value = mock_database + mock_db.session.query.return_value.get.return_value = mock_database mock_transpile.return_value = "SELECT * FROM `foo`" @@ -95,7 +95,7 @@ def test_transpile_virtual_dataset_sql_no_change(mock_transpile, mock_db): """Test when transpilation returns same SQL (no dialect differences).""" mock_database = MagicMock() mock_database.db_engine_spec.engine = "postgresql" - mock_db.session.get.return_value = mock_database + mock_db.session.query.return_value.get.return_value = mock_database original_sql = "SELECT * FROM foo" mock_transpile.return_value = original_sql @@ -118,7 +118,7 @@ def test_transpile_virtual_dataset_sql_error_fallback(mock_transpile, mock_db): mock_database = MagicMock() mock_database.db_engine_spec.engine = "mysql" - mock_db.session.get.return_value = mock_database + mock_db.session.query.return_value.get.return_value = mock_database mock_transpile.side_effect = QueryClauseValidationException("Parse error") @@ -140,7 +140,7 @@ def test_transpile_virtual_dataset_sql_postgres_to_duckdb(mock_transpile, mock_d """Test transpilation from PostgreSQL to DuckDB.""" mock_database = MagicMock() mock_database.db_engine_spec.engine = "duckdb" - mock_db.session.get.return_value = mock_database + mock_db.session.query.return_value.get.return_value = mock_database original_sql = """ SELECT DATE_TRUNC('month', created_at) AS month, COUNT(*) AS cnt @@ -173,7 +173,7 @@ def test_transpile_virtual_dataset_sql_postgres_to_clickhouse(mock_transpile, mo """ mock_database = MagicMock() mock_database.db_engine_spec.engine = "clickhouse" - mock_db.session.get.return_value = mock_database + mock_db.session.query.return_value.get.return_value = mock_database # PostgreSQL syntax original_sql = "SELECT DATE_TRUNC('month', created_at) AS month FROM orders" @@ -201,7 +201,7 @@ def test_transpile_virtual_dataset_sql_postgres_to_mysql(mock_transpile, mock_db """ mock_database = MagicMock() mock_database.db_engine_spec.engine = "mysql" - mock_db.session.get.return_value = mock_database + mock_db.session.query.return_value.get.return_value = mock_database # PostgreSQL syntax with :: casting original_sql = "SELECT created_at::DATE AS date_only FROM orders" @@ -226,7 +226,7 @@ def test_transpile_virtual_dataset_sql_postgres_to_sqlite(mock_transpile, mock_d """Test transpilation from PostgreSQL to SQLite.""" mock_database = MagicMock() mock_database.db_engine_spec.engine = "sqlite" - mock_db.session.get.return_value = mock_database + mock_db.session.query.return_value.get.return_value = mock_database original_sql = "SELECT * FROM orders WHERE created_at > NOW() - INTERVAL '7 days'" transpiled_sql = ( diff --git a/tests/unit_tests/databases/filters_test.py b/tests/unit_tests/databases/filters_test.py index 6a6cc665346..022dc04f979 100644 --- a/tests/unit_tests/databases/filters_test.py +++ b/tests/unit_tests/databases/filters_test.py @@ -119,5 +119,5 @@ def test_database_filter(mocker: MockerFixture) -> None: ) assert ( str(compiled_query) - == "SELECT dbs.id, dbs.verbose_name, dbs.database_name, dbs.sqlalchemy_uri, dbs.password, dbs.cache_timeout, dbs.select_as_create_table_as, dbs.expose_in_sqllab, dbs.configuration_method, dbs.allow_run_async, dbs.allow_file_upload, dbs.allow_ctas, dbs.allow_cvas, dbs.allow_dml, dbs.force_ctas_schema, dbs.extra, dbs.encrypted_extra, dbs.impersonate_user, dbs.server_cert, dbs.is_managed_externally, dbs.external_url, dbs.created_on, dbs.changed_on, dbs.created_by_fk, dbs.changed_by_fk, dbs.uuid, ssh_tunnels_1.id AS id_1, ssh_tunnels_1.database_id, ssh_tunnels_1.server_address, ssh_tunnels_1.server_port, ssh_tunnels_1.username, ssh_tunnels_1.password AS password_1, ssh_tunnels_1.private_key, ssh_tunnels_1.private_key_password, ssh_tunnels_1.server_host_key, ssh_tunnels_1.created_on AS created_on_1, ssh_tunnels_1.changed_on AS changed_on_1, ssh_tunnels_1.created_by_fk AS created_by_fk_1, ssh_tunnels_1.changed_by_fk AS changed_by_fk_1, ssh_tunnels_1.extra_json, ssh_tunnels_1.uuid AS uuid_1 \nFROM dbs LEFT OUTER JOIN ssh_tunnels AS ssh_tunnels_1 ON dbs.id = ssh_tunnels_1.database_id \nWHERE ('[' || dbs.database_name || '].(id:' || CAST(dbs.id AS VARCHAR) || ')') IN ('[my_db].(id:42)', '[my_other_db].(id:43)') OR dbs.database_name IN ('my_db', 'my_other_db', 'third_db')" # noqa: E501 + == "SELECT dbs.uuid, dbs.created_on, dbs.changed_on, dbs.id, dbs.verbose_name, dbs.database_name, dbs.sqlalchemy_uri, dbs.password, dbs.cache_timeout, dbs.select_as_create_table_as, dbs.expose_in_sqllab, dbs.configuration_method, dbs.allow_run_async, dbs.allow_file_upload, dbs.allow_ctas, dbs.allow_cvas, dbs.allow_dml, dbs.force_ctas_schema, dbs.extra, dbs.encrypted_extra, dbs.impersonate_user, dbs.server_cert, dbs.is_managed_externally, dbs.external_url, dbs.created_by_fk, dbs.changed_by_fk, ssh_tunnels_1.uuid AS uuid_1, ssh_tunnels_1.created_on AS created_on_1, ssh_tunnels_1.changed_on AS changed_on_1, ssh_tunnels_1.extra_json, ssh_tunnels_1.id AS id_1, ssh_tunnels_1.database_id, ssh_tunnels_1.server_address, ssh_tunnels_1.server_port, ssh_tunnels_1.username, ssh_tunnels_1.password AS password_1, ssh_tunnels_1.private_key, ssh_tunnels_1.private_key_password, ssh_tunnels_1.server_host_key, ssh_tunnels_1.created_by_fk AS created_by_fk_1, ssh_tunnels_1.changed_by_fk AS changed_by_fk_1 \nFROM dbs LEFT OUTER JOIN ssh_tunnels AS ssh_tunnels_1 ON dbs.id = ssh_tunnels_1.database_id \nWHERE '[' || dbs.database_name || '].(id:' || CAST(dbs.id AS VARCHAR) || ')' IN ('[my_db].(id:42)', '[my_other_db].(id:43)') OR dbs.database_name IN ('my_db', 'my_other_db', 'third_db')" # noqa: E501 ) diff --git a/tests/unit_tests/db_engine_specs/test_snowflake.py b/tests/unit_tests/db_engine_specs/test_snowflake.py index 3f166b22650..27f8f7d0216 100644 --- a/tests/unit_tests/db_engine_specs/test_snowflake.py +++ b/tests/unit_tests/db_engine_specs/test_snowflake.py @@ -216,30 +216,21 @@ def test_adjust_engine_params_fully_qualified() -> None: url = make_url("snowflake://user:pass@account/database_name/default") uri = SnowflakeEngineSpec.adjust_engine_params(url, {})[0] - assert ( - uri.render_as_string(hide_password=False) - == "snowflake://user:pass@account/database_name/default" - ) + assert str(uri) == "snowflake://user:pass@account/database_name/default" uri = SnowflakeEngineSpec.adjust_engine_params( url, {}, schema="new_schema", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "snowflake://user:pass@account/database_name/new_schema" - ) + assert str(uri) == "snowflake://user:pass@account/database_name/new_schema" uri = SnowflakeEngineSpec.adjust_engine_params( url, {}, catalog="new_catalog", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "snowflake://user:pass@account/new_catalog/default" - ) + assert str(uri) == "snowflake://user:pass@account/new_catalog/default" uri = SnowflakeEngineSpec.adjust_engine_params( url, @@ -247,10 +238,7 @@ def test_adjust_engine_params_fully_qualified() -> None: catalog="new_catalog", schema="new_schema", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "snowflake://user:pass@account/new_catalog/new_schema" - ) + assert str(uri) == "snowflake://user:pass@account/new_catalog/new_schema" def test_adjust_engine_params_catalog_only() -> None: @@ -262,30 +250,21 @@ def test_adjust_engine_params_catalog_only() -> None: url = make_url("snowflake://user:pass@account/database_name") uri = SnowflakeEngineSpec.adjust_engine_params(url, {})[0] - assert ( - uri.render_as_string(hide_password=False) - == "snowflake://user:pass@account/database_name" - ) + assert str(uri) == "snowflake://user:pass@account/database_name" uri = SnowflakeEngineSpec.adjust_engine_params( url, {}, schema="new_schema", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "snowflake://user:pass@account/database_name/new_schema" - ) + assert str(uri) == "snowflake://user:pass@account/database_name/new_schema" uri = SnowflakeEngineSpec.adjust_engine_params( url, {}, catalog="new_catalog", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "snowflake://user:pass@account/new_catalog" - ) + assert str(uri) == "snowflake://user:pass@account/new_catalog" uri = SnowflakeEngineSpec.adjust_engine_params( url, @@ -293,10 +272,7 @@ def test_adjust_engine_params_catalog_only() -> None: catalog="new_catalog", schema="new_schema", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "snowflake://user:pass@account/new_catalog/new_schema" - ) + assert str(uri) == "snowflake://user:pass@account/new_catalog/new_schema" def test_get_default_catalog() -> None: diff --git a/tests/unit_tests/db_engine_specs/test_trino.py b/tests/unit_tests/db_engine_specs/test_trino.py index b08118ea42d..b7cb70d5374 100644 --- a/tests/unit_tests/db_engine_specs/test_trino.py +++ b/tests/unit_tests/db_engine_specs/test_trino.py @@ -844,30 +844,21 @@ def test_adjust_engine_params_fully_qualified() -> None: url = make_url("trino://user:pass@localhost:8080/system/default") uri = TrinoEngineSpec.adjust_engine_params(url, {})[0] - assert ( - uri.render_as_string(hide_password=False) - == "trino://user:pass@localhost:8080/system/default" - ) + assert str(uri) == "trino://user:pass@localhost:8080/system/default" uri = TrinoEngineSpec.adjust_engine_params( url, {}, schema="new_schema", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "trino://user:pass@localhost:8080/system/new_schema" - ) + assert str(uri) == "trino://user:pass@localhost:8080/system/new_schema" uri = TrinoEngineSpec.adjust_engine_params( url, {}, catalog="new_catalog", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "trino://user:pass@localhost:8080/new_catalog/default" - ) + assert str(uri) == "trino://user:pass@localhost:8080/new_catalog/default" uri = TrinoEngineSpec.adjust_engine_params( url, @@ -875,10 +866,7 @@ def test_adjust_engine_params_fully_qualified() -> None: catalog="new_catalog", schema="new_schema", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "trino://user:pass@localhost:8080/new_catalog/new_schema" - ) + assert str(uri) == "trino://user:pass@localhost:8080/new_catalog/new_schema" def test_adjust_engine_params_catalog_only() -> None: @@ -890,30 +878,21 @@ def test_adjust_engine_params_catalog_only() -> None: url = make_url("trino://user:pass@localhost:8080/system") uri = TrinoEngineSpec.adjust_engine_params(url, {})[0] - assert ( - uri.render_as_string(hide_password=False) - == "trino://user:pass@localhost:8080/system" - ) + assert str(uri) == "trino://user:pass@localhost:8080/system" uri = TrinoEngineSpec.adjust_engine_params( url, {}, schema="new_schema", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "trino://user:pass@localhost:8080/system/new_schema" - ) + assert str(uri) == "trino://user:pass@localhost:8080/system/new_schema" uri = TrinoEngineSpec.adjust_engine_params( url, {}, catalog="new_catalog", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "trino://user:pass@localhost:8080/new_catalog" - ) + assert str(uri) == "trino://user:pass@localhost:8080/new_catalog" uri = TrinoEngineSpec.adjust_engine_params( url, @@ -921,10 +900,7 @@ def test_adjust_engine_params_catalog_only() -> None: catalog="new_catalog", schema="new_schema", )[0] - assert ( - uri.render_as_string(hide_password=False) - == "trino://user:pass@localhost:8080/new_catalog/new_schema" - ) + assert str(uri) == "trino://user:pass@localhost:8080/new_catalog/new_schema" @pytest.mark.parametrize( diff --git a/tests/unit_tests/extensions/test_sqlalchemy.py b/tests/unit_tests/extensions/test_sqlalchemy.py index 735dc701eb5..f0ac735b0c2 100644 --- a/tests/unit_tests/extensions/test_sqlalchemy.py +++ b/tests/unit_tests/extensions/test_sqlalchemy.py @@ -17,7 +17,6 @@ # pylint: disable=redefined-outer-name, import-outside-toplevel, unused-argument import os -import re from collections.abc import Iterator from typing import TYPE_CHECKING @@ -38,15 +37,6 @@ if TYPE_CHECKING: from superset.models.core import Database -def _normalize_sqla_doc_link(message: str) -> str: - """ - Replace the SQLAlchemy error-doc URL's version segment (e.g. ``/e/20/``) - with a placeholder so assertions don't need updating every time - SQLAlchemy bumps its minor version. - """ - return re.sub(r"/e/\d+/", "/e/XX/", message) - - @pytest.fixture def database1(session: Session) -> Iterator["Database"]: from superset.models.core import Database @@ -292,11 +282,11 @@ def test_dml( conn.execute( text("""INSERT INTO "database2.table2" (a, b) VALUES (3, 'thirty')""") ) - assert _normalize_sqla_doc_link(str(excinfo.value).strip()) == ( + assert str(excinfo.value).strip() == ( "(shillelagh.exceptions.ProgrammingError) DML not enabled in database " '"database2"\n[SQL: INSERT INTO "database2.table2" (a, b) ' "VALUES (3, 'thirty')]\n(Background on this error at: " - "https://sqlalche.me/e/XX/f405)" + "https://sqlalche.me/e/14/f405)" ) @@ -384,10 +374,10 @@ def test_allowed_dbs(mocker: MockerFixture, app_context: None, table1: None) -> with engine.connect() as conn: with pytest.raises(ProgrammingError) as excinfo: conn.execute(text('SELECT * FROM "database2.table2"')) - assert _normalize_sqla_doc_link(str(excinfo.value)) == ( + assert str(excinfo.value) == ( """ (shillelagh.exceptions.ProgrammingError) Unsupported table: database2.table2 [SQL: SELECT * FROM "database2.table2"] -(Background on this error at: https://sqlalche.me/e/XX/f405) +(Background on this error at: https://sqlalche.me/e/14/f405) """.strip() ) diff --git a/tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py index 88926808506..b85ed85e0ca 100644 --- a/tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py +++ b/tests/unit_tests/mcp_service/chart/tool/test_get_chart_data.py @@ -1058,7 +1058,7 @@ class TestChartDataCommandValidation: "superset.common.query_context_factory.QueryContextFactory" ) as mock_factory, ): - mock_db.session.get.return_value = mock_dataset + mock_db.session.query.return_value.get.return_value = mock_dataset mock_factory.return_value.create.return_value = MagicMock() from superset.mcp_service.chart.preview_utils import ( @@ -1106,7 +1106,7 @@ class TestChartDataCommandValidation: "superset.common.query_context_factory.QueryContextFactory" ) as mock_factory, ): - mock_db.session.get.return_value = mock_dataset + mock_db.session.query.return_value.get.return_value = mock_dataset mock_factory.return_value.create.return_value = MagicMock() from superset.mcp_service.chart.preview_utils import ( diff --git a/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py b/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py index ac38ed55ba5..9271d7d070f 100644 --- a/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py +++ b/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py @@ -40,7 +40,6 @@ from superset.mcp_service.dashboard.tool.add_chart_to_existing_dashboard import from superset.mcp_service.dashboard.tool.generate_dashboard import ( _generate_title_from_charts, ) -from superset.models.dashboard import Dashboard as _RealDashboard from superset.utils import json logging.basicConfig(level=logging.DEBUG) @@ -168,24 +167,6 @@ def _setup_generate_dashboard_mocks( mock_dashboard_cls.return_value = dashboard mock_find_by_id.return_value = dashboard - # `generate_dashboard` builds its re-fetch eager-load options with - # `subqueryload(Dashboard.slices).subqueryload(Slice.editors)` etc. - # against this same patched `Dashboard` class. SQLAlchemy 2.0 validates - # loader-path arguments eagerly and raises `ArgumentError` ("Wildcard - # token cannot be followed by another entity") when given a plain - # MagicMock attribute instead of a real `InstrumentedAttribute` -- - # SQLAlchemy 1.4 didn't validate this eagerly, so the same mock chain - # silently worked before. Copy over the real class-level relationship - # attributes (captured at module import time, before `Dashboard` gets - # patched, since `from ... import Dashboard` done here would just - # return the mock itself) so `subqueryload`/`joinedload` construction - # sees genuine mapped attributes while `Dashboard(...)` instantiation - # (used to create new dashboards) still returns the mocked `dashboard` - # object. - mock_dashboard_cls.slices = _RealDashboard.slices - mock_dashboard_cls.editors = _RealDashboard.editors - mock_dashboard_cls.tags = _RealDashboard.tags - # Prevent Subject DB queries during dashboard creation. # The mock is started here and will be cleaned up by patch.stopall() # or when the test process ends. diff --git a/tests/unit_tests/sql_lab_test.py b/tests/unit_tests/sql_lab_test.py index bb9a2c1cbb8..c554a95f405 100644 --- a/tests/unit_tests/sql_lab_test.py +++ b/tests/unit_tests/sql_lab_test.py @@ -496,17 +496,7 @@ def test_get_sql_results_oauth2(mocker: MockerFixture, app) -> None: "OAuth2 required" ) - # `limit` and `select_as_cta_used` must match the real `Query` model's - # defaults (nullable Integer -> None, Boolean default=False) so that - # `apply_limit` -- called unconditionally before the mocked OAuth2 error - # is ever reached -- doesn't try to compare an unconfigured MagicMock - # against an int. - query = mocker.MagicMock( - select_as_cta=False, - select_as_cta_used=False, - limit=None, - database=database, - ) + query = mocker.MagicMock(select_as_cta=False, database=database) mocker.patch("superset.sql_lab.get_query", return_value=query) payload = get_sql_results(query_id=1, rendered_query="SELECT 1") @@ -519,9 +509,7 @@ def test_get_sql_results_oauth2(mocker: MockerFixture, app) -> None: assert error["error_type"] == SupersetErrorType.OAUTH2_REDIRECT assert error["level"] == ErrorLevel.WARNING assert error["extra"]["tab_id"] == "fb11f528-6eba-4a8a-837e-6b0d39ee9187" - assert ( - error["extra"]["redirect_uri"] == "http://example.com/api/v1/database/oauth2/" - ) + assert error["extra"]["redirect_uri"] == "http://localhost/api/v1/database/oauth2/" # Parse the OAuth2 authorization URL and verify components individually, # since the JWT state and PKCE code_challenge are computed deterministically @@ -534,7 +522,7 @@ def test_get_sql_results_oauth2(mocker: MockerFixture, app) -> None: params = parse_qs(url.query) assert params["scope"] == ["refresh_token session:role:USERADMIN"] assert params["response_type"] == ["code"] - assert params["redirect_uri"] == ["http://example.com/api/v1/database/oauth2/"] + assert params["redirect_uri"] == ["http://localhost/api/v1/database/oauth2/"] assert params["client_id"] == ["my_client_id"] assert params["code_challenge_method"] == ["S256"]