mikebridge commented on code in PR #44269:
URL: https://github.com/apache/superset/pull/44269#discussion_r4039348290
##########
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:
Good catch — addressed in 67d690b4f5156c6bfcef2cac7ae838fc65028243.
Semantic-view metadata failures are isolated per group, logged with the view ID
and counted once, while healthy tables/views remain available. The catch
deliberately re-raises SQL serialization errors rather than broadening that
contract. Four before-change controls covered columns/metrics with
KeyError/RuntimeError, including duplicate charts and colliding IDs; all failed
before the fix. The final focused file passes 19 tests, and required pre-commit
(including MyPy) passed. New-head CI is running, not yet certified green.
##########
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:
Good catch — addressed in 67d690b4f5156c6bfcef2cac7ae838fc65028243 with the
SemanticView import and nested joinedload of semantic_layer. Cold-session
SQLite tests use the tool's actual query options with one and three distinct
layers, then assert serialization adds no SELECTs. Both failed before the fix
(5 versus 4 and 7 versus 4 SELECTs); the final focused file passes 19 tests and
required pre-commit/MyPy passed. New-head CI is running.
--
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]
