aminghadersohi commented on code in PR #44681:
URL: https://github.com/apache/superset/pull/44681#discussion_r4113630477
##########
superset/mcp_service/chart/tool/list_charts.py:
##########
@@ -63,6 +63,52 @@
_DEFAULT_LIST_CHARTS_REQUEST = ListChartsRequest()
+# Relationships that resolve a chart's live datasource. ``datasource_name`` is
+# read through them rather than from the stored ``Slice.datasource_name``
+# column, which can be stale after a dataset rename or a chart re-point.
+_LIVE_DATASOURCE_RELATIONSHIPS = ("table", "semantic_view")
+
+
+class _ChartListCore(ModelListCore[ChartList]):
+ """List core that loads the live datasource whenever its name is requested.
+
+ Requesting only plain columns makes the DAO return row tuples, which cannot
+ reach the ``table`` / ``semantic_view`` relationships. Adding them to the
+ DAO load list switches it to full model instances with the relationships
+ eager-loaded, so the live name costs no per-chart query.
+ """
+
+ def _call_dao_list(
+ self,
+ filters: Any,
+ order_column: str,
+ order_direction: str,
+ page: int,
+ page_size: int,
+ search: str | None,
+ columns_to_load: list[str],
+ custom_filters: dict[str, Any] | None = None,
+ ) -> tuple[list[Any], int]:
+ if "datasource_name" in columns_to_load:
+ columns_to_load = [
+ *columns_to_load,
+ *(
+ rel
+ for rel in _LIVE_DATASOURCE_RELATIONSHIPS
+ if rel not in columns_to_load
+ ),
+ ]
Review Comment:
VALID — fixed in 115530452e38732b9ab9554f715185c968af9670. list_charts
replaces stored-name predicates with type-guarded live table/semantic-view name
expressions before DAO counting and pagination, retaining query/saved-query
names. Real-DAO regression tests cover current vs stale names, both datasource
types, operators, missing datasets and pagination; these failed before the fix
and pass afterward. Existing metadata filter permission checks are unchanged.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -128,7 +129,16 @@ class ChartInfo(BaseModel):
"fall back to viz_type when this field is null."
),
)
- datasource_name: str | None = Field(None, description="Datasource name")
+ datasource_id: int | None = Field(
+ None, description="ID of the dataset (or semantic view) the chart
queries"
+ )
Review Comment:
VALID — fixed in 115530452e38732b9ab9554f715185c968af9670. Added
datasource_id to DEFAULT_GET_CHART_INFO_COLUMNS. The new default-selection
regression failed before the fix; default tool-response tests verify the live
ID for authorized callers and null for callers without data-model metadata
access.
##########
superset/mcp_service/chart/schemas.py:
##########
@@ -549,7 +559,41 @@ def extract_filters_from_form_data(
)
-def serialize_chart_object(chart: ChartLike | None) -> ChartInfo | None:
+def resolve_chart_datasource_id(chart: Any) -> int | None:
+ """Return the ID of the datasource a chart is joined to."""
+ datasource_id = getattr(chart, "datasource_id", None)
+ if isinstance(datasource_id, int) and not isinstance(datasource_id, bool):
+ return datasource_id
+ return None
+
+
+def resolve_chart_datasource_name(chart: Any) -> str | None:
+ """Return the chart's datasource name, read from the live datasource.
+
+ ``Slice.datasource_name`` is a stored, denormalized column that is not
+ refreshed when a dataset is renamed or a chart is re-pointed, so it can
+ name a table the chart no longer queries. ``Slice.datasource_name_text``
+ resolves the name through the type-guarded ``table`` / ``semantic_view``
+ relationships instead, and yields ``None`` when the datasource no longer
+ exists. Objects without that resolver (row tuples, lightweight stand-ins)
+ fall back to the stored value.
+ """
+ resolver = getattr(chart, "datasource_name_text", None)
+ if callable(resolver):
+ live_name = resolver()
+ if live_name is None or isinstance(live_name, str):
+ return live_name
Review Comment:
VALID — fixed in 115530452e38732b9ab9554f715185c968af9670. The shared
resolve_chart_datasource_name helper uses the stored name for query/saved_query
charts instead of invoking the table/semantic-view resolver. Chart
serialization regressions for both types failed before and pass after. Missing
tables still return no stale name, and get_chart_info/get_chart_sql retain
DatasetNotAccessible for deleted datasets.
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1795,7 +1808,10 @@ def serialize_chart_summary(
id=chart_id,
slice_name=getattr(chart, "slice_name", None),
viz_type=getattr(chart, "viz_type", None),
- datasource_name=getattr(chart, "datasource_name", None)
+ datasource_id=resolve_chart_datasource_id(chart)
+ if include_data_model_metadata
+ else None,
+ datasource_name=resolve_chart_datasource_name(chart)
if include_data_model_metadata
else None,
Review Comment:
VALID — fixed in 115530452e38732b9ab9554f715185c968af9670. Dashboard
summaries use the corrected shared resolve_chart_datasource_name helper,
preserving stored names for query/saved_query charts. Dashboard serialization
regressions for both types failed before and pass after; table/semantic-view
live names and permission-based identifier/name redaction remain covered.
--
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]
