mikebridge commented on code in PR #45393:
URL: https://github.com/apache/superset/pull/45393#discussion_r4327072714


##########
superset-core/src/superset_core/semantic_layers/layer.py:
##########
@@ -80,6 +80,9 @@ class SemanticLayer(ABC, Generic[ConfigT, SemanticViewT]):
     Abstract base class for semantic layers.
     """
 
+    # Parsed settings read by the host when requesting the runtime schema.
+    # Providers supply this instance attribute; the SDK does not initialize it.
+    configuration: ConfigT

Review Comment:
   Confirmed with MyPy 1.15: the raw-dict error is intentional, but the 
read-only-property override error restricts storage choices too. I’ve left the 
declaration unchanged in this patch and tracked the compatibility question as a 
follow-up: explicitly require writable parsed configuration, or support 
read-only properties through a revised typing contract.



##########
superset-core/tests/semantic_layers/example_provider.py:
##########
@@ -0,0 +1,167 @@
+# 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.
+
+"""A small SDK-only provider; import under semantic_layer_registration in 
tests."""
+
+from typing import Any
+
+import pyarrow as pa
+from pydantic import BaseModel
+from superset_core.semantic_layers.context import (
+    SemanticCredentials,
+    SemanticExecutionContext,
+)
+from superset_core.semantic_layers.decorators import semantic_layer
+from superset_core.semantic_layers.errors import (
+    SemanticQueryErrorCode,
+    SemanticQueryRejectedError,
+)
+from superset_core.semantic_layers.layer import SemanticLayer
+from superset_core.semantic_layers.types import (
+    Dimension,
+    Filter,
+    Metric,
+    SemanticQuery,
+    SemanticResult,
+)
+from superset_core.semantic_layers.view import SemanticView
+
+
+class ExampleConfig(BaseModel):
+    """Parsed connection settings exposed through the layer."""
+
+    label: str = "Example"
+
+
+class ExampleView(SemanticView):
+    """An in-memory dimension-only view with no optional capabilities or 
cache."""
+
+    def __init__(self, cache_token: str | None = None) -> None:
+        """Capture the discovery generation for this view."""
+        self._cache_token: str | None = cache_token
+
+    @property
+    def metadata_cache_token(self) -> str | None:
+        """Echo the captured generation without retaining a shared cache."""
+        return self._cache_token
+
+    def uid(self) -> str:
+        """Return stable identity independently of display labels."""
+        return "example"
+
+    def get_dimensions(self) -> set[Dimension]:
+        """Expose one member whose private ID differs from its selection 
name."""
+        return {Dimension(id="private-category", name="category", 
type=pa.string())}
+
+    def get_metrics(self) -> set[Metric]:
+        """This example has no metrics."""
+        return set()
+
+    def get_values(
+        self, dimension: Dimension, filters: set[Filter] | None = None
+    ) -> SemanticResult:
+        """Return complete distinct values named after the requested member."""
+        if dimension not in self.get_dimensions():
+            raise 
SemanticQueryRejectedError(SemanticQueryErrorCode.INVALID_QUERY)
+        if filters:
+            raise 
SemanticQueryRejectedError(SemanticQueryErrorCode.INVALID_FILTER)

Review Comment:
   Fixed in 3f5d0a9b0194. The minimal example refuses filtering with 
UNSUPPORTED_QUERY, rather than blaming a valid search filter. Added a 
regression using the host’s WHERE/LIKE search shape; the unfiltered result and 
unknown-dimension behavior are preserved.



##########
superset-core/src/superset_core/semantic_layers/testing.py:
##########
@@ -0,0 +1,56 @@
+# 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.
+
+"""Opt-in pytest support for provider tests without a Superset application.
+
+Load with ``pytest_plugins = ["superset_core.semantic_layers.testing"]``.
+This module requires pytest; importing the provider SDK itself does not.
+"""
+
+from collections.abc import Callable
+from typing import TypeVar
+
+import pytest
+
+from superset_core.semantic_layers import decorators
+
+_T = TypeVar("_T")
+
+
+def _registration(
+    id: str, name: str, description: str | None = None
+) -> Callable[[_T], _T]:
+    """Leave the decorated class intact without registering it with any 
host."""
+
+    def decorate(provider: _T) -> _T:
+        return provider

Review Comment:
   Fixed in 3f5d0a9b0194. The stub sets name and description, including the 
default None description. Tests check both values and class identity; registry 
writes and ID prefixing remain outside the fixture.



##########
superset-core/src/superset_core/semantic_layers/testing.py:
##########
@@ -0,0 +1,56 @@
+# 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.
+
+"""Opt-in pytest support for provider tests without a Superset application.
+
+Load with ``pytest_plugins = ["superset_core.semantic_layers.testing"]``.
+This module requires pytest; importing the provider SDK itself does not.
+"""
+
+from collections.abc import Callable
+from typing import TypeVar
+
+import pytest

Review Comment:
   Added in 3f5d0a9b0194: apache-superset-core[testing] installs pytest through 
an optional extra. The module and author guide point to it; pytest is not a 
default dependency.



-- 
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]

Reply via email to