aminghadersohi commented on code in PR #43737:
URL: https://github.com/apache/superset/pull/43737#discussion_r3971236754
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -389,6 +420,22 @@ def _validate_update_against_dataset(
}
)
+ try:
+ merged_gantt_config = validate_gantt_form_data(
Review Comment:
Fixed in ea43f4bbbb2f966871a7ba7296145bf6e3410fdb (normal merge of upstream
master 70346a51b6999a6624406a1c801f0fc197947505; no force push).
`GanttChartConfig.adapt_native_form_data` now removes `slice_name` as
saved-chart transport metadata. The visualization-control allowlist is
otherwise unchanged.
Regression evidence:
- The four parametrized
`test_gantt_update_preview_matches_would_be_persisted_state` cases now pass the
**complete** `_build_preview_form_data` output to `validate_gantt_form_data`
before stripping metadata for the persistence comparison. All four failed
before the fix with `Unknown field 'slice_name'`.
- `test_default_gantt_update_validates_named_preview_before_caching` calls
`update_chart` with `generate_preview` omitted, keeps the real merged-state
validation gate, and checks successful validation/caching of `slice_name`,
`slice_id`, and `datasource`. Only external boundaries/compile execution are
mocked; persistence must not run. This default-flow regression failed before
the fix and passes afterward.
- `test_gantt_transport_name_does_not_allow_unknown_controls` still rejects
a misspelled visualization control alongside valid naming metadata.
All six targeted cases pass; full chart suite: **1,769 passed**, including
upstream Gauge renderer coverage. Full PR-delta pre-commit passed (MyPy, Ruff,
Pylint), as did changed-Python compilation and diff checks. The merge preserves
upstream Gauge/shared-update behavior alongside Gantt's existing
filter/presentation regressions. Broad-suite and exact-head required CI results
will be recorded in the PR body. No new review run requested.
Final validation on `924100c73c46c300a7be9212898dd9952b339640`: **4,163
MCP/common-query tests passed**, full PR static checks passed, and **all 13
exact-head required CI checks SUCCESS**. The follow-up commit fixes runner apt
setup; the Gantt fix remains unchanged. The PR is open and non-draft, and your
review has not been dismissed.
--
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]
