aminghadersohi commented on code in PR #44269:
URL: https://github.com/apache/superset/pull/44269#discussion_r4014778854
##########
superset/mcp_service/dashboard/tool/get_dashboard_datasets.py:
##########
@@ -113,6 +118,7 @@ async def get_dashboard_datasets(
slice_dataset.subqueryload(SqlaTable.columns),
slice_dataset.subqueryload(SqlaTable.metrics),
slice_dataset.joinedload(SqlaTable.database),
+ subqueryload(Dashboard.slices).subqueryload(Slice.semantic_view),
Review Comment:
`SemanticView.semantic_layer` is lazy by default and always read (layer
uuid/name, and `raise_for_access`'s layer-perm fallback), leaving one SELECT
per view. Chaining `.joinedload(SemanticView.semantic_layer)` also needs a
`SemanticView` import above, so it is not one span.
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -2381,21 +2438,27 @@ def dashboard_datasets_serializer(dashboard:
"Dashboard") -> DashboardDatasets:
datasets: List[DashboardDatasetSummary] = []
inaccessible_count: int = 0
- for slices in slices_by_datasource.values():
+ for (_, source_type), slices in slices_by_datasource.items():
+ kind: Literal["table", "semantic_view"] = (
+ "semantic_view" if source_type == DatasourceType.SEMANTIC_VIEW
else "table"
+ )
+ relationship_name: str = (
+ "semantic_view" if kind == "semantic_view" else "datasource"
+ )
datasource = next(
(
- getattr(slc, "datasource", None)
+ getattr(slc, relationship_name, None)
for slc in slices
- if getattr(slc, "datasource", None) is not None
+ if getattr(slc, relationship_name, None) is not None
),
None,
)
if datasource is None:
continue
- if not has_dataset_access(datasource):
+ if not _has_dashboard_dataset_access(datasource, kind):
inaccessible_count += 1
continue
- datasets.append(_serialize_dashboard_dataset(datasource, len(slices)))
+ datasets.append(_serialize_dashboard_dataset(datasource, len(slices),
kind))
Review Comment:
`SemanticView.columns`/`.metrics` are provider-backed, so an unregistered
layer type (`KeyError`) or a provider error fails the entire tool with
`InternalError`, losing the dashboard's healthy SQL datasets too.
`list_metrics.py:224` degrades per-view for exactly this reason.
```suggestion
try:
summary = _serialize_dashboard_dataset(datasource, len(slices),
kind)
except Exception as exc: # noqa: BLE001
# External registry may be empty in OSS - degrade gracefully.
logger.warning("Could not serialize datasource: %s", exc)
inaccessible_count += 1
continue
datasets.append(summary)
```
--
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]
