mikebridge commented on code in PR #44370:
URL: https://github.com/apache/superset/pull/44370#discussion_r4052634917
##########
superset/common/query_object.py:
##########
@@ -405,6 +406,38 @@ def validate(
) -> QueryObjectValidationError | None:
"""Validate query object"""
try:
+ if self.datasource and self.datasource.type == "semantic_view":
+ try:
+ cast(
+ "SemanticView", self.datasource
+ ).implementation.validate_selection_version(
+ self.extras.get("semantic_selection_version")
+ )
Review Comment:
Fixed in 18877f01d2. Implementation resolution is outside the
selection-version ValueError catch; malformed configuration now retains its
configuration exception instead of reporting legacy selections. Red-first
regression added. Final validation: 807 backend tests, 69 Jest tests and all
required branch-file hooks (including MyPy/full frontend types) pass.
@aminghadersohi please re-review.
##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -182,17 +186,34 @@ def _resolve_external_view(
error_type="AccessDenied",
)
- display_name = view.name
- valid_columns = {c.column_name for c in view.columns}
- valid_dttm_columns = {c.column_name for c in view.columns if c.is_dttm}
- valid_metrics = {m.metric_name for m in view.metrics}
+ try:
+ view.implementation.validate_selection_version(
+ request.semantic_selection_version
+ )
Review Comment:
Fixed in 18877f01d2. MCP separates configuration resolution from
selection-version validation and returns a fixed ConfigurationError message for
malformed JSON, without raw parse text. Missing/stale versions remain
ValidationError. Red-first regression and full affected checks pass.
@aminghadersohi please re-review.
##########
superset/datasource/api.py:
##########
@@ -751,6 +751,7 @@ def _build_query_dict(
time_range=payload["time_range"],
time_grain=payload["time_grain"],
grain_column=grain_column,
+
semantic_selection_version=payload.get("semantic_selection_version"),
Review Comment:
Fixed in 18877f01d2 using Mike’s option 2: versioned semantic views return
HTTP 200 with suggestions_status=unavailable_versioned_view and an empty result
BEFORE any cache read/provider execution. Both callers show a manual-entry
fallback; Matrixify preserves existing values. Tables/unversioned views keep
their existing behavior. Red-first route/cache and frontend manual-selection
regressions pass. Option 1 (version/provenance-aware API and both callers) is
tracked in [SC-121456](https://app.shortcut.com/preset/story/121456), owned by
Mike/Tooligans, epic89652, Triage. @aminghadersohi please re-review.
##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -220,6 +220,7 @@ def _build_query_dict(
dimensions=request.dimensions,
filters=[{"col": f.col, "op": f.op, "val": f.val} for f in
request.filters],
time_range=request.time_range,
+ semantic_selection_version=request.semantic_selection_version,
Review Comment:
Updated in 18877f01d2: stale/missing selection versions are ValidationError,
while malformed configuration JSON gets a sanitized ConfigurationError. Both
cases have regression coverage; 807 backend tests,69 frontend tests and
required hooks pass.
--
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]
