fitzee commented on code in PR #45393:
URL: https://github.com/apache/superset/pull/45393#discussion_r4321898909
##########
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:
non-blocking: Declaring this as a plain (writable) attribute narrows which
storage choices type-check. A provider that exposes `configuration` as a
read-only `@property` (e.g. parsing lazily from a stored dict) now fails mypy
with `Cannot override writeable attribute with read-only property [override]`.
Reproduced with mypy 1.15 `--strict` against this branch. A provider that
stores the raw dict under this name now gets an `[assignment]` error. The
second error is the point of the change. The first one contradicts the
description's "providers retain their own storage and construction choices".
Either document that it must be a plain instance attribute, or say providers
using a property should expect the override error.
##########
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:
nit: `apache-superset-core` doesn't declare pytest anywhere. A `testing =
["pytest"]` optional extra in `superset-core/pyproject.toml` would give
provider authors a pinned, discoverable install target, instead of an
`ImportError` explained only by the module docstring.
##########
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:
nit: The host decorator (`core_api_injection.semantic_layer_impl`)
unconditionally sets `cls.name`, `cls.description` and
`cls._semantic_layer_id`, and the host reads `cls.name`/`cls.description` when
listing types. Under this stub those attributes are missing. Provider code or
tests that read `self.name` (logging, error text) will pass in production and
raise `AttributeError` in the fixture, or the reverse. Setting
`name`/`description` here, without the registry or prefixing, would keep the
fixture closer to the host's class shape. Otherwise, list them in the "does not
emulate" sentence.
##########
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:
non-blocking: The host always sends a filter here when a user types in a
filter-value search box. `SemanticView.values` (models.py) builds a `LIKE
%search%` narrowing `Filter` for string dimensions. So a provider copied from
this example rejects every search keystroke, and the user sees
`INVALID_FILTER`'s host text: "A semantic query filter is invalid. Check its
operator and values". That wording blames the user's input. `UNSUPPORTED_QUERY`
("does not support this query. Remove unsupported filters…") is the accurate
code for a filter the provider chooses not to implement. Better still, apply
the single `LIKE` containment filter, since that's the only filter shape the
host sends to `get_values` and it's trivial over an in-memory list.
--
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]
