From a36bbf8ffd370ac97ccafa75c6e53b90da080a44 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 26 Nov 2025 14:46:44 -0500 Subject: [PATCH] Small fixes --- .../chart/data/streaming_export_command.py | 9 +------- superset/commands/dataset/export.py | 5 +--- superset/common/query_actions.py | 2 +- superset/common/query_context_processor.py | 2 +- superset/connectors/sqla/models.py | 2 +- superset/explorables/base.py | 23 +++++++++++++++---- superset/models/helpers.py | 4 +++- superset/thumbnails/digest.py | 7 +----- superset/utils/core.py | 4 ---- tests/unit_tests/utils/map_type_tests.py | 5 +++- 10 files changed, 31 insertions(+), 32 deletions(-) diff --git a/superset/commands/chart/data/streaming_export_command.py b/superset/commands/chart/data/streaming_export_command.py index 28764c3ae81..7dec6bc1d41 100644 --- a/superset/commands/chart/data/streaming_export_command.py +++ b/superset/commands/chart/data/streaming_export_command.py @@ -68,14 +68,7 @@ class StreamingCSVExportCommand(BaseStreamingCSVExportCommand): query_obj = self._query_context.queries[0] sql_query = datasource.get_query_str(query_obj.to_dict()) - # Chart export is SQL-specific, so we check for BaseDatasource - from superset.connectors.sqla.models import BaseDatasource - - database = ( - datasource.database if isinstance(datasource, BaseDatasource) else None - ) - - return sql_query, database + return sql_query, getattr(datasource, "database", None) def _get_row_limit(self) -> int | None: """ diff --git a/superset/commands/dataset/export.py b/superset/commands/dataset/export.py index e298a8f58f1..354df539f89 100644 --- a/superset/commands/dataset/export.py +++ b/superset/commands/dataset/export.py @@ -84,10 +84,7 @@ class ExportDatasetsCommand(ExportModelsCommand): # SQLAlchemy returns column names as quoted_name objects which PyYAML cannot # serialize. Convert all keys to regular strings to fix YAML serialization. try: - from sqlalchemy.sql.elements import quoted_name - - if any(isinstance(key, quoted_name) for key in payload.keys()): - payload = {str(key): value for key, value in payload.items()} + payload = {str(key): value for key, value in payload.items()} except ImportError: pass diff --git a/superset/common/query_actions.py b/superset/common/query_actions.py index d59804ff613..c7760ebd4ea 100644 --- a/superset/common/query_actions.py +++ b/superset/common/query_actions.py @@ -73,7 +73,7 @@ def _get_query( _: bool, ) -> dict[str, Any]: datasource = _get_datasource(query_context, query_obj) - result = {"language": getattr(datasource, "query_language", None)} + result = {"language": datasource.query_language} try: result["query"] = datasource.get_query_str(query_obj.to_dict()) except QueryObjectValidationError as err: diff --git a/superset/common/query_context_processor.py b/superset/common/query_context_processor.py index eec2275a23b..8b2713590d3 100644 --- a/superset/common/query_context_processor.py +++ b/superset/common/query_context_processor.py @@ -483,6 +483,6 @@ class QueryContextProcessor: query.validate() if self._qc_datasource.type == DatasourceType.QUERY: - security_manager.raise_for_access(datasource=self._qc_datasource) + security_manager.raise_for_access(query=self._qc_datasource) else: security_manager.raise_for_access(query_context=self._query_context) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 8767a7336fa..a0fb9c686f5 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -225,7 +225,7 @@ class BaseDatasource( This allows each datasource to override caching, while falling back to database-level defaults when appropriate. """ - if self._cache_timeout: + if self._cache_timeout is not None: return self._cache_timeout # database should always be set, but that's not true for v0 import diff --git a/superset/explorables/base.py b/superset/explorables/base.py index eb1d678d565..b58fe2136e2 100644 --- a/superset/explorables/base.py +++ b/superset/explorables/base.py @@ -110,6 +110,21 @@ class Explorable(Protocol): :return: Type identifier string """ + @property + def metrics(self) -> list[Any]: + """ + List of metric metadata objects. + + Each object should provide at minimum: + - metric_name: str - the metric's name + - expression: str - the metric's calculation expression + + Used for validation, autocomplete, and query building. + + :return: List of metric metadata objects + """ + + # TODO: rename to dimensions @property def columns(self) -> list[Any]: """ @@ -125,6 +140,7 @@ class Explorable(Protocol): :return: List of column metadata objects """ + # TODO: remove and use columns instead @property def column_names(self) -> list[str]: """ @@ -136,6 +152,7 @@ class Explorable(Protocol): :return: List of column name strings """ + # TODO: use TypedDict for return type @property def data(self) -> dict[str, Any]: """ @@ -289,11 +306,7 @@ class Explorable(Protocol): """ # ========================================================================= - # Optional Properties - # ========================================================================= - - # ========================================================================= - # Required Methods + # Drilling # ========================================================================= def has_drill_by_columns(self, column_names: list[str]) -> bool: diff --git a/superset/models/helpers.py b/superset/models/helpers.py index 01498cedd9f..945819ec152 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -467,7 +467,9 @@ class ImportExportMixin(UUIDMixin): if parent_ref: parent_excludes = {c.name for c in parent_ref.local_columns} dict_rep = { - c.name: getattr(self, c.name) + # Convert c.name to str to handle SQLAlchemy's quoted_name type + # which is not YAML-serializable + str(c.name): getattr(self, c.name) for c in cls.__table__.columns # type: ignore if ( c.name in export_fields diff --git a/superset/thumbnails/digest.py b/superset/thumbnails/digest.py index 41389e15944..1cdf8d3a642 100644 --- a/superset/thumbnails/digest.py +++ b/superset/thumbnails/digest.py @@ -61,7 +61,6 @@ def _adjust_string_with_rls( """ Add the RLS filters to the unique string based on current executor. """ - from superset.connectors.sqla.models import BaseDatasource user = ( security_manager.find_user(executor) @@ -72,11 +71,7 @@ def _adjust_string_with_rls( stringified_rls = "" with override_user(user): for datasource in datasources: - if ( - datasource - and isinstance(datasource, BaseDatasource) - and datasource.is_rls_supported - ): + if datasource and getattr(datasource, "is_rls_supported", False): rls_filters = datasource.get_sqla_row_level_filters() if len(rls_filters) > 0: diff --git a/superset/utils/core.py b/superset/utils/core.py index b4058165b27..05c7f51cf34 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -1677,10 +1677,6 @@ def get_metric_type_from_column( from superset.connectors.sqla.models import SqlMetric - # Explorable datasources may not have metrics attribute - if datasource is None or not hasattr(datasource, "metrics"): - return "" - metric: SqlMetric = next( (metric for metric in datasource.metrics if metric.metric_name == column), SqlMetric(metric_name=""), diff --git a/tests/unit_tests/utils/map_type_tests.py b/tests/unit_tests/utils/map_type_tests.py index a91fe1abf0a..b076880e147 100644 --- a/tests/unit_tests/utils/map_type_tests.py +++ b/tests/unit_tests/utils/map_type_tests.py @@ -16,6 +16,8 @@ # from unittest.mock import MagicMock, patch +import pytest + from superset.connectors.sqla.models import SqlMetric from superset.utils.core import ( get_metric_type_from_column, @@ -60,7 +62,8 @@ def test_column_is_none(): def test_datasource_is_none(): datasource = None column = "my_column" - assert get_metric_type_from_column(column, datasource) == "" + with pytest.raises(AttributeError): + get_metric_type_from_column(column, datasource) def test_none_input():