mikebridge commented on code in PR #44835:
URL: https://github.com/apache/superset/pull/44835#discussion_r4208800660
##########
superset/semantic_layers/api.py:
##########
@@ -731,10 +736,19 @@ def runtime_schema(self, uuid: str) -> FlaskResponse:
return self.response_400(message=f"Unknown type: {layer.type}")
try:
- schema = cls.get_runtime_schema(
- layer.implementation.configuration, # type:
ignore[attr-defined]
- runtime_data,
- )
+ schema: dict[str, Any]
+ if metadata_binding.participates(layer):
+ adapter: MetadataRefreshAdapter | None = (
+ layer.implementation.metadata_refresh
+ )
+ if adapter is None:
+ raise MetadataRefreshError("configuration")
Review Comment:
Fixed in fd44ff9dae. Runtime schema and compatibility now use the shared
typed-error mapper, and runtime schema rethrows the typed error through its
broad exception boundary. Real authorized discovery regressions verify
localized 503/504 responses instead of 400/500.
##########
superset/common/query_context_processor.py:
##########
@@ -478,7 +557,18 @@ def _annotation_cache_context(self, query_obj:
QueryObject) -> dict[str, Any]:
if annotation_datasource
else None
)
- return {"user_id": get_user_id(), "source_rls": source_rls}
+ metadata_datasource: Datasource | None = (
+ chart.resolved_datasource if chart else None
Review Comment:
Fixed in fd44ff9dae. The annotation key uses the datasource recorded in the
saved query context, matching capture and execution. Resolving that identity
does not build the query or run discovery. Regressions cover mismatched
advertised/saved sources, cache invalidation, and discovery-free key
construction.
##########
tests/unit_tests/semantic_layers/metadata_identity_test.py:
##########
@@ -0,0 +1,715 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Exercise metadata identity through the real QueryContext result cache."""
+
+from __future__ import annotations
+
+from collections.abc import Callable
+from datetime import datetime, timedelta
+from typing import Any, cast
+from unittest.mock import Mock, patch
+from uuid import UUID
+
+import pyarrow as pa
+import pytest
+from flask import Flask
+from flask_caching import Cache
+from superset_core.semantic_layers.metadata import MetadataRefreshErrorCategory
+from superset_core.semantic_layers.types import Metric, SemanticQuery,
SemanticResult
+
+from superset.common.chart_data import ChartDataResultFormat,
ChartDataResultType
+from superset.common.query_context import QueryContext
+from superset.common.query_object import QueryObject
+from superset.common.utils import query_cache_manager
+from superset.connectors.sqla.models import BaseDatasource
+from superset.constants import CacheRegion
+from superset.explorables.base import Explorable
+from superset.semantic_layers.models import SemanticLayer, SemanticView
+from superset.semantic_layers.registry import registry
+from tests.unit_tests.semantic_layers.metadata_contract_test import (
+ LegacyLayer,
+ LegacyView,
+)
+
+
+class ResultView(LegacyView):
+ def __init__(self, token: str, value: int) -> None:
+ self.token: str = token
+ self.value: int = value
+ self.calls: int = 0
+
+ @property
+ def metadata_cache_token(self) -> str:
+ return self.token
+
+ def get_metrics(self) -> set[Metric]:
+ return {Metric("orders", "orders", pa.int64(), "orders")}
+
+ def get_table(self, query: SemanticQuery) -> SemanticResult:
+ self.calls += 1
+ return SemanticResult([], pa.table({"orders": [self.value]}))
+
+
+class RefreshLayer(LegacyLayer):
+ @classmethod
+ def supports_metadata_refresh(cls, configuration: dict[str, Any]) -> bool:
+ return True
+
+
+def view_for(provider: ResultView) -> SemanticView:
+ layer: SemanticLayer = SemanticLayer(
+ uuid=UUID("00000000-0000-0000-0000-000000000011"),
+ type="cache-test",
+ configuration="{}",
+ )
+ view: SemanticView = SemanticView(
+ id=11,
+ uuid=UUID("00000000-0000-0000-0000-000000000012"),
+ name="orders",
+ configuration="{}",
+ semantic_layer=layer,
+ changed_on=datetime(2026, 1, 1),
+ cache_timeout=300,
+ )
+ view.__dict__["_fixture_implementation"] = provider
+ return view
+
+
+def context_for(view: Explorable) -> tuple[QueryContext, QueryObject]:
+ query: QueryObject = QueryObject(
+ datasource=cast(BaseDatasource, view), metrics=["orders"], row_limit=10
+ )
+ context: QueryContext = QueryContext(
+ datasource=view,
+ queries=[query],
+ slice_=None,
+ form_data=None,
+ result_type=ChartDataResultType.FULL,
+ result_format=ChartDataResultFormat.JSON,
+ cache_values={},
+ )
+ return context, query
+
+
+def test_same_name_and_discovery_after_refresh_cannot_reuse_old_query_result(
Review Comment:
Added both regressions in fd44ff9dae: views differing only in stored
configuration get separate compatibility answers and uncached query results,
with catalog token, name, selection and changed_on held constant. Removing
configuration from the key makes both tests fail; production hashing already
included it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
