aminghadersohi commented on code in PR #41611: URL: https://github.com/apache/superset/pull/41611#discussion_r3508235421
########## superset/mcp_service/semantic_layer/tool/get_compatible_dimensions.py: ########## @@ -0,0 +1,214 @@ +# 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. + +"""MCP tool: get_compatible_dimensions + +Returns dimensions compatible with the current metric/dimension selection. +""" + +import logging + +from fastmcp import Context +from superset_core.mcp.decorators import tool, ToolAnnotations + +from superset.extensions import event_logger +from superset.mcp_service.privacy import ( + DATA_MODEL_METADATA_ERROR_TYPE, + requires_data_model_metadata_access, + user_can_view_data_model_metadata, +) +from superset.mcp_service.semantic_layer.schemas import ( + CompatibleDimensionsResponse, + DimensionInfo, + GetCompatibleDimensionsRequest, + SemanticLayerError, +) + +logger = logging.getLogger(__name__) + + +@tool( + tags=["data", "semantic"], + class_permission_name="Dataset", + annotations=ToolAnnotations( + title="Get compatible dimensions", + readOnlyHint=True, + destructiveHint=False, + ), +) +@requires_data_model_metadata_access +async def get_compatible_dimensions( + request: GetCompatibleDimensionsRequest, + ctx: Context, +) -> CompatibleDimensionsResponse | SemanticLayerError: + """Return dimensions compatible with the current metric/dimension selection. + + Used to drive progressive disclosure in query builders: after the user + selects one or more metrics (and optionally some dimensions), this tool + returns the dimensions that can validly be added without breaking the + underlying query. + + Provide exactly one of ``dataset_id`` (built-in) or ``view_id`` (external). + + For built-in datasets, returns all groupby-enabled columns from the same + dataset as the selected metrics, filtered to those whose datasets intersect + with all selected metrics. Review Comment: Fixed — the docstring now says: "For built-in datasets, returns all groupby-enabled columns from the dataset. SQL datasets have no semantic compatibility constraints between metrics and dimensions, so all groupby columns are always returned regardless of the selected metrics." This matches the implementation, which intentionally ignores `selected_metrics`/`selected_dimensions` for the built-in path (SQL `GROUP BY` has no per-metric dimension constraints, unlike external semantic views which do enforce compatibility via `view.get_compatible_dimensions()`). ########## superset/mcp_service/semantic_layer/tool/get_compatible_dimensions.py: ########## @@ -0,0 +1,214 @@ +# 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. + +"""MCP tool: get_compatible_dimensions + +Returns dimensions compatible with the current metric/dimension selection. +""" + +import logging + +from fastmcp import Context +from superset_core.mcp.decorators import tool, ToolAnnotations + +from superset.extensions import event_logger +from superset.mcp_service.privacy import ( + DATA_MODEL_METADATA_ERROR_TYPE, + requires_data_model_metadata_access, + user_can_view_data_model_metadata, +) +from superset.mcp_service.semantic_layer.schemas import ( + CompatibleDimensionsResponse, + DimensionInfo, + GetCompatibleDimensionsRequest, + SemanticLayerError, +) + +logger = logging.getLogger(__name__) + + +@tool( + tags=["data", "semantic"], + class_permission_name="Dataset", + annotations=ToolAnnotations( + title="Get compatible dimensions", + readOnlyHint=True, + destructiveHint=False, + ), +) +@requires_data_model_metadata_access +async def get_compatible_dimensions( + request: GetCompatibleDimensionsRequest, + ctx: Context, +) -> CompatibleDimensionsResponse | SemanticLayerError: + """Return dimensions compatible with the current metric/dimension selection. + + Used to drive progressive disclosure in query builders: after the user + selects one or more metrics (and optionally some dimensions), this tool + returns the dimensions that can validly be added without breaking the + underlying query. + + Provide exactly one of ``dataset_id`` (built-in) or ``view_id`` (external). + + For built-in datasets, returns all groupby-enabled columns from the same + dataset as the selected metrics, filtered to those whose datasets intersect + with all selected metrics. + + For external semantic views, delegates to the view's + ``get_compatible_dimensions`` implementation. + + Example: + ```json + { + "selected_metrics": ["revenue"], + "selected_dimensions": [], + "view_id": 5 + } + ``` + """ + await ctx.info( + "Getting compatible dimensions: dataset_id=%s, view_id=%s, " + "metrics=%s, dims=%s" + % ( + request.dataset_id, + request.view_id, + request.selected_metrics, + request.selected_dimensions, + ) + ) + + if not user_can_view_data_model_metadata(): + return SemanticLayerError.create( + error="You don't have permission to access dataset details for your role.", + error_type=DATA_MODEL_METADATA_ERROR_TYPE, + ) + + if request.dataset_id is None and request.view_id is None: + return SemanticLayerError.create( + error="Provide either dataset_id (built-in) or view_id (external).", + error_type="ValidationError", + ) + if request.dataset_id is not None and request.view_id is not None: + return SemanticLayerError.create( + error="Provide only one of dataset_id or view_id, not both.", + error_type="ValidationError", + ) + + try: + # ------------------------------------------------------------------ + # Built-in dataset path + # ------------------------------------------------------------------ + if request.dataset_id is not None: + from sqlalchemy.orm import subqueryload + + from superset.connectors.sqla.models import SqlaTable + from superset.daos.dataset import DatasetDAO + + dataset_id: int = request.dataset_id + with event_logger.log_context( + action="mcp.get_compatible_dimensions.builtin" + ): + dataset = DatasetDAO.find_by_id( + dataset_id, + query_options=[ + subqueryload(SqlaTable.columns), + subqueryload(SqlaTable.metrics), + ], + ) + + if dataset is None: + return SemanticLayerError.create( + error=f"No dataset found with id: {request.dataset_id}.", + error_type="NotFound", + ) + + # For built-in datasets all groupby columns are always compatible; + # there's no per-metric compatibility constraint at the SQL level. + dims = [ + DimensionInfo( + name=col.column_name, + verbose_name=col.verbose_name or None, + description=col.description or None, + type=col.type or None, + is_dttm=bool(col.is_dttm), + groupby=bool(col.groupby), + filterable=bool(col.filterable), + source="builtin", + ) + for col in dataset.columns + if col.groupby + ] + + await ctx.info("Compatible dimensions (builtin): count=%d" % len(dims)) + return CompatibleDimensionsResponse( + compatible_dimensions=dims, + source="builtin", + ) + + # ------------------------------------------------------------------ + # External semantic view path + # ------------------------------------------------------------------ + from superset.daos.semantic_layer import SemanticViewDAO + + view_id: int = request.view_id # type: ignore[assignment] + with event_logger.log_context(action="mcp.get_compatible_dimensions.external"): + view = SemanticViewDAO.find_by_id(view_id) + + if view is None: + return SemanticLayerError.create( + error=f"No semantic view found with id: {view_id}.", + error_type="NotFound", + ) + + compatible_names = view.get_compatible_dimensions( + request.selected_metrics, + request.selected_dimensions, + ) Review Comment: Fixed — added `view.raise_for_access()` before reading `view.columns`/`view.get_compatible_dimensions()`, catching `SupersetSecurityException` and returning a dedicated `AccessDenied` error type (see `get_compatible_dimensions.py` lines 179-185). On collapsing AccessDenied into NotFound to avoid existence disclosure: we checked the precedent in this codebase — the SemanticView's own REST API (`superset/semantic_layers/api.py`) already distinguishes `response_404()` (not found) from `response_403()` (access denied via `raise_for_access`) rather than merging them. Keeping `AccessDenied` distinct from `NotFound` here mirrors that existing convention for this exact resource type, so we kept it as-is rather than introducing a different disclosure model just for the MCP surface. ########## superset/mcp_service/semantic_layer/tool/get_compatible_dimensions.py: ########## @@ -0,0 +1,214 @@ +# 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. + +"""MCP tool: get_compatible_dimensions + +Returns dimensions compatible with the current metric/dimension selection. +""" + +import logging + +from fastmcp import Context +from superset_core.mcp.decorators import tool, ToolAnnotations + +from superset.extensions import event_logger +from superset.mcp_service.privacy import ( + DATA_MODEL_METADATA_ERROR_TYPE, + requires_data_model_metadata_access, + user_can_view_data_model_metadata, +) +from superset.mcp_service.semantic_layer.schemas import ( + CompatibleDimensionsResponse, + DimensionInfo, + GetCompatibleDimensionsRequest, + SemanticLayerError, +) + +logger = logging.getLogger(__name__) + + +@tool( + tags=["data", "semantic"], + class_permission_name="Dataset", + annotations=ToolAnnotations( + title="Get compatible dimensions", + readOnlyHint=True, + destructiveHint=False, + ), +) +@requires_data_model_metadata_access +async def get_compatible_dimensions( + request: GetCompatibleDimensionsRequest, + ctx: Context, +) -> CompatibleDimensionsResponse | SemanticLayerError: + """Return dimensions compatible with the current metric/dimension selection. + + Used to drive progressive disclosure in query builders: after the user + selects one or more metrics (and optionally some dimensions), this tool + returns the dimensions that can validly be added without breaking the + underlying query. + + Provide exactly one of ``dataset_id`` (built-in) or ``view_id`` (external). + + For built-in datasets, returns all groupby-enabled columns from the same + dataset as the selected metrics, filtered to those whose datasets intersect + with all selected metrics. + + For external semantic views, delegates to the view's + ``get_compatible_dimensions`` implementation. + + Example: + ```json + { + "selected_metrics": ["revenue"], + "selected_dimensions": [], + "view_id": 5 + } + ``` + """ + await ctx.info( + "Getting compatible dimensions: dataset_id=%s, view_id=%s, " + "metrics=%s, dims=%s" + % ( + request.dataset_id, + request.view_id, + request.selected_metrics, + request.selected_dimensions, + ) + ) + + if not user_can_view_data_model_metadata(): + return SemanticLayerError.create( + error="You don't have permission to access dataset details for your role.", + error_type=DATA_MODEL_METADATA_ERROR_TYPE, + ) + + if request.dataset_id is None and request.view_id is None: + return SemanticLayerError.create( + error="Provide either dataset_id (built-in) or view_id (external).", + error_type="ValidationError", + ) + if request.dataset_id is not None and request.view_id is not None: + return SemanticLayerError.create( + error="Provide only one of dataset_id or view_id, not both.", + error_type="ValidationError", + ) + + try: + # ------------------------------------------------------------------ + # Built-in dataset path + # ------------------------------------------------------------------ + if request.dataset_id is not None: + from sqlalchemy.orm import subqueryload + + from superset.connectors.sqla.models import SqlaTable + from superset.daos.dataset import DatasetDAO + + dataset_id: int = request.dataset_id + with event_logger.log_context( + action="mcp.get_compatible_dimensions.builtin" + ): + dataset = DatasetDAO.find_by_id( + dataset_id, + query_options=[ + subqueryload(SqlaTable.columns), + subqueryload(SqlaTable.metrics), + ], + ) + + if dataset is None: + return SemanticLayerError.create( + error=f"No dataset found with id: {request.dataset_id}.", + error_type="NotFound", + ) + + # For built-in datasets all groupby columns are always compatible; + # there's no per-metric compatibility constraint at the SQL level. + dims = [ + DimensionInfo( + name=col.column_name, + verbose_name=col.verbose_name or None, + description=col.description or None, + type=col.type or None, + is_dttm=bool(col.is_dttm), + groupby=bool(col.groupby), + filterable=bool(col.filterable), + source="builtin", + ) + for col in dataset.columns + if col.groupby + ] + + await ctx.info("Compatible dimensions (builtin): count=%d" % len(dims)) + return CompatibleDimensionsResponse( + compatible_dimensions=dims, + source="builtin", + ) + + # ------------------------------------------------------------------ + # External semantic view path + # ------------------------------------------------------------------ + from superset.daos.semantic_layer import SemanticViewDAO + + view_id: int = request.view_id # type: ignore[assignment] + with event_logger.log_context(action="mcp.get_compatible_dimensions.external"): + view = SemanticViewDAO.find_by_id(view_id) + + if view is None: + return SemanticLayerError.create( + error=f"No semantic view found with id: {view_id}.", + error_type="NotFound", + ) + + compatible_names = view.get_compatible_dimensions( + request.selected_metrics, + request.selected_dimensions, + ) + + # Enrich with full column metadata + all_cols = {col.column_name: col for col in view.columns} + dims = [ + DimensionInfo( + name=name, + verbose_name=all_cols[name].verbose_name if name in all_cols else None, + description=all_cols[name].description if name in all_cols else None, + type=all_cols[name].type if name in all_cols else None, + is_dttm=all_cols[name].is_dttm if name in all_cols else False, + groupby=all_cols[name].groupby if name in all_cols else True, + filterable=all_cols[name].filterable if name in all_cols else True, + source="external", + ) + for name in compatible_names + ] + + await ctx.info( + "Compatible dimensions (external view id=%d): count=%d" + % (view.id, len(dims)) + ) + return CompatibleDimensionsResponse( + compatible_dimensions=dims, + source="external", + ) + + except Exception as exc: + logger.exception( + "Unexpected error in get_compatible_dimensions: %s: %s", + type(exc).__name__, + str(exc), + ) + await ctx.error("Unexpected error: %s: %s" % (type(exc).__name__, str(exc))) + raise Review Comment: Fixed — the outer `except Exception` now returns a typed `SemanticLayerError(error_type="InternalError")` instead of re-raising. See `get_compatible_dimensions.py` lines 217-227. ########## superset/mcp_service/semantic_layer/tool/get_compatible_metrics.py: ########## @@ -0,0 +1,214 @@ +# 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. + +"""MCP tool: get_compatible_metrics + +Returns metrics compatible with the current dimension/metric selection. +""" + +import logging + +from fastmcp import Context +from superset_core.mcp.decorators import tool, ToolAnnotations + +from superset.extensions import event_logger +from superset.mcp_service.privacy import ( + DATA_MODEL_METADATA_ERROR_TYPE, + requires_data_model_metadata_access, + user_can_view_data_model_metadata, +) +from superset.mcp_service.semantic_layer.schemas import ( + CompatibleMetricsResponse, + GetCompatibleMetricsRequest, + MetricInfo, + SemanticLayerError, +) + +logger = logging.getLogger(__name__) + + +@tool( + tags=["data", "semantic"], + class_permission_name="Dataset", + annotations=ToolAnnotations( + title="Get compatible metrics", + readOnlyHint=True, + destructiveHint=False, + ), +) +@requires_data_model_metadata_access +async def get_compatible_metrics( + request: GetCompatibleMetricsRequest, + ctx: Context, +) -> CompatibleMetricsResponse | SemanticLayerError: + """Return metrics compatible with the current dimension/metric selection. + + Used to progressively refine a query: given a set of already-selected + metrics and dimensions, returns the additional metrics that can be + combined without breaking the underlying semantic constraints. + + Provide exactly one of ``dataset_id`` (built-in) or ``view_id`` (external). + + For built-in datasets, all metrics from the dataset are considered + compatible (SQL GROUP BY imposes no metric-level constraints). + + For external semantic views, delegates to the view's + ``get_compatible_metrics`` implementation. + + Example: + ```json + { + "selected_metrics": [], + "selected_dimensions": ["region"], + "view_id": 5 + } + ``` + """ + await ctx.info( + "Getting compatible metrics: dataset_id=%s, view_id=%s, " + "metrics=%s, dims=%s" + % ( + request.dataset_id, + request.view_id, + request.selected_metrics, + request.selected_dimensions, + ) + ) + + if not user_can_view_data_model_metadata(): + return SemanticLayerError.create( + error="You don't have permission to access dataset details for your role.", + error_type=DATA_MODEL_METADATA_ERROR_TYPE, + ) + + if request.dataset_id is None and request.view_id is None: + return SemanticLayerError.create( + error="Provide either dataset_id (built-in) or view_id (external).", + error_type="ValidationError", + ) + if request.dataset_id is not None and request.view_id is not None: + return SemanticLayerError.create( + error="Provide only one of dataset_id or view_id, not both.", + error_type="ValidationError", + ) + + try: + # ------------------------------------------------------------------ + # Built-in dataset path + # ------------------------------------------------------------------ + if request.dataset_id is not None: + from sqlalchemy.orm import subqueryload + + from superset.connectors.sqla.models import SqlaTable + from superset.daos.dataset import DatasetDAO + + with event_logger.log_context(action="mcp.get_compatible_metrics.builtin"): + dataset = DatasetDAO.find_by_id( + request.dataset_id, + query_options=[ + subqueryload(SqlaTable.columns), + subqueryload(SqlaTable.metrics), + ], + ) + + if dataset is None: + return SemanticLayerError.create( + error=f"No dataset found with id: {request.dataset_id}.", + error_type="NotFound", + ) + + # All metrics on a SQL dataset are always mutually compatible. + compatible = [ + MetricInfo( + name=m.metric_name, + verbose_name=m.verbose_name or None, + description=m.description or None, + expression=m.expression or None, + d3format=m.d3format or None, + warning_text=m.warning_text or None, + source="builtin", + dataset_id=dataset.id, + dataset_name=dataset.table_name, + ) + for m in dataset.metrics + ] + + await ctx.info("Compatible metrics (builtin): count=%d" % len(compatible)) + return CompatibleMetricsResponse( + compatible_metrics=compatible, + source="builtin", + ) + + # ------------------------------------------------------------------ + # External semantic view path + # ------------------------------------------------------------------ + from superset.daos.semantic_layer import SemanticViewDAO + + view_id: int = request.view_id # type: ignore[assignment] + with event_logger.log_context(action="mcp.get_compatible_metrics.external"): + view = SemanticViewDAO.find_by_id(view_id) + + if view is None: + return SemanticLayerError.create( + error=f"No semantic view found with id: {view_id}.", + error_type="NotFound", + ) + + compatible_names = view.get_compatible_metrics( + request.selected_metrics, + request.selected_dimensions, + ) Review Comment: Fixed — added `view.raise_for_access()` before reading `view.metrics`/`view.get_compatible_metrics()`, catching `SupersetSecurityException` and returning a dedicated `AccessDenied` error type. Same rationale as the `get_compatible_dimensions` thread for keeping `AccessDenied` distinct from `NotFound` (matches `superset/semantic_layers/api.py`'s existing 403-vs-404 convention). See `get_compatible_metrics.py` lines 172-178. ########## superset/mcp_service/semantic_layer/tool/get_compatible_metrics.py: ########## @@ -0,0 +1,214 @@ +# 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. + +"""MCP tool: get_compatible_metrics + +Returns metrics compatible with the current dimension/metric selection. +""" + +import logging + +from fastmcp import Context +from superset_core.mcp.decorators import tool, ToolAnnotations + +from superset.extensions import event_logger +from superset.mcp_service.privacy import ( + DATA_MODEL_METADATA_ERROR_TYPE, + requires_data_model_metadata_access, + user_can_view_data_model_metadata, +) +from superset.mcp_service.semantic_layer.schemas import ( + CompatibleMetricsResponse, + GetCompatibleMetricsRequest, + MetricInfo, + SemanticLayerError, +) + +logger = logging.getLogger(__name__) + + +@tool( + tags=["data", "semantic"], + class_permission_name="Dataset", + annotations=ToolAnnotations( + title="Get compatible metrics", + readOnlyHint=True, + destructiveHint=False, + ), +) +@requires_data_model_metadata_access +async def get_compatible_metrics( + request: GetCompatibleMetricsRequest, + ctx: Context, +) -> CompatibleMetricsResponse | SemanticLayerError: + """Return metrics compatible with the current dimension/metric selection. + + Used to progressively refine a query: given a set of already-selected + metrics and dimensions, returns the additional metrics that can be + combined without breaking the underlying semantic constraints. + + Provide exactly one of ``dataset_id`` (built-in) or ``view_id`` (external). + + For built-in datasets, all metrics from the dataset are considered + compatible (SQL GROUP BY imposes no metric-level constraints). + + For external semantic views, delegates to the view's + ``get_compatible_metrics`` implementation. + + Example: + ```json + { + "selected_metrics": [], + "selected_dimensions": ["region"], + "view_id": 5 + } + ``` + """ + await ctx.info( + "Getting compatible metrics: dataset_id=%s, view_id=%s, " + "metrics=%s, dims=%s" + % ( + request.dataset_id, + request.view_id, + request.selected_metrics, + request.selected_dimensions, + ) + ) + + if not user_can_view_data_model_metadata(): + return SemanticLayerError.create( + error="You don't have permission to access dataset details for your role.", + error_type=DATA_MODEL_METADATA_ERROR_TYPE, + ) + + if request.dataset_id is None and request.view_id is None: + return SemanticLayerError.create( + error="Provide either dataset_id (built-in) or view_id (external).", + error_type="ValidationError", + ) + if request.dataset_id is not None and request.view_id is not None: + return SemanticLayerError.create( + error="Provide only one of dataset_id or view_id, not both.", + error_type="ValidationError", + ) + + try: + # ------------------------------------------------------------------ + # Built-in dataset path + # ------------------------------------------------------------------ + if request.dataset_id is not None: + from sqlalchemy.orm import subqueryload + + from superset.connectors.sqla.models import SqlaTable + from superset.daos.dataset import DatasetDAO + + with event_logger.log_context(action="mcp.get_compatible_metrics.builtin"): + dataset = DatasetDAO.find_by_id( + request.dataset_id, + query_options=[ + subqueryload(SqlaTable.columns), + subqueryload(SqlaTable.metrics), + ], + ) + + if dataset is None: + return SemanticLayerError.create( + error=f"No dataset found with id: {request.dataset_id}.", + error_type="NotFound", + ) + + # All metrics on a SQL dataset are always mutually compatible. + compatible = [ + MetricInfo( + name=m.metric_name, + verbose_name=m.verbose_name or None, + description=m.description or None, + expression=m.expression or None, + d3format=m.d3format or None, + warning_text=m.warning_text or None, + source="builtin", + dataset_id=dataset.id, + dataset_name=dataset.table_name, + ) + for m in dataset.metrics + ] + + await ctx.info("Compatible metrics (builtin): count=%d" % len(compatible)) + return CompatibleMetricsResponse( + compatible_metrics=compatible, + source="builtin", + ) + + # ------------------------------------------------------------------ + # External semantic view path + # ------------------------------------------------------------------ + from superset.daos.semantic_layer import SemanticViewDAO + + view_id: int = request.view_id # type: ignore[assignment] + with event_logger.log_context(action="mcp.get_compatible_metrics.external"): + view = SemanticViewDAO.find_by_id(view_id) + + if view is None: + return SemanticLayerError.create( + error=f"No semantic view found with id: {view_id}.", + error_type="NotFound", + ) + + compatible_names = view.get_compatible_metrics( + request.selected_metrics, + request.selected_dimensions, + ) + + # Enrich with full metric metadata + all_metrics_map = {m.metric_name: m for m in view.metrics} + compatible = [ + MetricInfo( + name=name, + description=( + all_metrics_map[name].description + if name in all_metrics_map + else None + ), + expression=( + all_metrics_map[name].expression + if name in all_metrics_map + else None + ), + source="external", + view_id=view.id, + view_name=view.name, + ) + for name in compatible_names + ] + + await ctx.info( + "Compatible metrics (external view id=%d): count=%d" + % (view.id, len(compatible)) + ) + return CompatibleMetricsResponse( + compatible_metrics=compatible, + source="external", + ) + + except Exception as exc: + logger.exception( + "Unexpected error in get_compatible_metrics: %s: %s", + type(exc).__name__, + str(exc), + ) + await ctx.error("Unexpected error: %s: %s" % (type(exc).__name__, str(exc))) + raise Review Comment: Fixed — the outer `except Exception` now returns a typed `SemanticLayerError(error_type="InternalError")` instead of re-raising. See `get_compatible_metrics.py` lines 216-226. -- 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]
