aminghadersohi commented on code in PR #44746:
URL: https://github.com/apache/superset/pull/44746#discussion_r4120975795
##########
superset/mcp_service/chart/plugins/gauge.py:
##########
@@ -193,3 +202,51 @@ def schema_error_hint(self) -> ChartGenerationError | None:
],
error_code="GAUGE_VALIDATION_ERROR",
)
+
+ def normalize_query_result(self, result: Any, form_data: Mapping[str,
Any]) -> Any:
+ from superset.mcp_service.chart.query_result import (
+ normalize_gauge_query_result,
+ )
+
+ return normalize_gauge_query_result(result, form_data)
+
+ def compile_row_limit(self, form_data: Mapping[str, Any]) -> int:
+ return min(10, int(form_data.get("row_limit") or 10))
Review Comment:
Fixed in 49b2e0a2012eedef0a5b3ad40aa3c8d3ec9c45ea. Guarded compile row-limit
conversion against TypeError, ValueError and OverflowError, and use the bounded
10-row fallback for malformed/nonpositive values while preserving valid small
limits. Parametrized persisted-value regression tests fail before the fix and
pass after it.
Validation: chart unit suite 2604 passed, 3 skipped; pre-commit on changed
files passed, including mypy and pylint.
##########
superset/mcp_service/chart/tool/update_chart_preview.py:
##########
@@ -195,21 +194,27 @@ def update_chart_preview( # noqa: C901
or (previous_form_data or {}).get("datasource_id")
or ""
).split("__", 1)[0]
+ plugin = get_registry().get(config.chart_type)
Review Comment:
Fixed in 49b2e0a2012eedef0a5b3ad40aa3c8d3ec9c45ea. Update lookups explicitly
include disabled registered plugins, preserving resolve_update_config and
dataset-rebind hooks. Form-data mapping also retains disabled update contracts,
and column extraction/normalization no longer skips these plugins. Parametrized
FastMCP tests cover disabled Treemap saved updates and cached preview updates,
including partial config completion, omissions, validation and compile checks;
they fail before the fix and pass afterward. Creation lookups remain filtered
by default.
Validation: chart unit suite 2604 passed, 3 skipped; pre-commit on changed
files passed, including mypy and pylint.
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -866,19 +869,26 @@ async def update_chart( # noqa: C901
# config is already a typed ChartConfig | None (validated by Pydantic)
try:
+ config_plugin = (
+ get_registry().get(request.config.chart_type)
+ if request.config is not None
+ else None
Review Comment:
Fixed in 49b2e0a2012eedef0a5b3ad40aa3c8d3ec9c45ea. Update lookups explicitly
include disabled registered plugins, preserving resolve_update_config and
dataset-rebind hooks. Form-data mapping also retains disabled update contracts,
and column extraction/normalization no longer skips these plugins. Parametrized
FastMCP tests cover disabled Treemap saved updates and cached preview updates,
including partial config completion, omissions, validation and compile checks;
they fail before the fix and pass afterward. Creation lookups remain filtered
by default.
Validation: chart unit suite 2604 passed, 3 skipped; pre-commit on changed
files passed, including mypy and pylint.
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -866,19 +867,26 @@ async def update_chart( # noqa: C901
# config is already a typed ChartConfig | None (validated by Pydantic)
try:
+ config_plugin = (
+ get_registry().get(request.config.chart_type)
+ if request.config is not None
+ else None
+ )
parsed_config = (
- resolve_treemap_update_config(
+ config_plugin.resolve_update_config(
Review Comment:
Fixed in 49b2e0a2012eedef0a5b3ad40aa3c8d3ec9c45ea. Update lookups explicitly
include disabled registered plugins, preserving resolve_update_config and
dataset-rebind hooks. Form-data mapping also retains disabled update contracts,
and column extraction/normalization no longer skips these plugins. Parametrized
FastMCP tests cover disabled Treemap saved updates and cached preview updates,
including partial config completion, omissions, validation and compile checks;
they fail before the fix and pass afterward. Creation lookups remain filtered
by default.
Validation: chart unit suite 2604 passed, 3 skipped; pre-commit on changed
files passed, including mypy and pylint.
##########
superset/mcp_service/chart/plugins/gantt.py:
##########
@@ -235,3 +243,66 @@ def schema_error_hint(self) -> ChartGenerationError | None:
],
error_code="GANTT_VALIDATION_ERROR",
)
+
+ def build_query_dicts(
+ self,
+ form_data: dict[str, Any],
+ *,
+ viz_type: str,
+ engine: str,
+ row_limit: int | None,
+ order_desc: bool | None,
+ ) -> list[dict[str, Any]] | None:
+ from superset.mcp_service.chart.chart_helpers import (
+ build_single_query_dict,
+ resolve_gantt_query_fields,
+ )
+
+ columns, metrics, orderby, series_columns = resolve_gantt_query_fields(
+ form_data
+ )
+ query = build_single_query_dict(
+ form_data, columns, metrics, row_limit=row_limit
+ )
+ query["orderby"] = orderby
+ query["series_columns"] = series_columns
Review Comment:
Fixed in 49b2e0a2012eedef0a5b3ad40aa3c8d3ec9c45ea. Preserved rejection of
malformed Gantt fields rather than silently falling back to a different query.
The plugin translates resolver ValueError into QueryObjectValidationError;
saved and unsaved chart-data handlers return a structured ValidationError with
the original field-specific message. Five regression cases (missing
start/end/category, oversized tooltip columns, malformed ordering) previously
returned DataError and now pass.
Validation: chart unit suite 2604 passed, 3 skipped; pre-commit on changed
files passed, including mypy and pylint.
--
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]
