aminghadersohi opened a new pull request, #39922:
URL: https://github.com/apache/superset/pull/39922
### SUMMARY
Introduces a `ChartTypePlugin` protocol and central registry that
consolidates chart-type-specific logic into a single plugin class per chart
type. Previously, adding a new chart type required synchronised edits to four
separate locations:
1. `schema_validator.py` — pre-validation dispatch
2. `dataset_validator.py` — column extraction
3. `chart_utils.py` — form_data mapping
4. `runtime/__init__.py` — runtime warning dispatch
With this change, each of the 7 supported chart types (`xy`, `table`, `pie`,
`pivot_table`, `mixed_timeseries`, `handlebars`, `big_number`) owns all five
responsibilities in a single plugin file under `chart/plugins/`.
**Key fixes included:**
- **5-type column validation gap**: `pie`, `pivot_table`,
`mixed_timeseries`, `handlebars`, and `big_number` charts previously skipped
dataset column validation entirely (silent false-pass). All 7 types now run
column existence and aggregation checks.
- **BigNumber trendline check**: Moved out of the monolithic dispatcher and
into `BigNumberChartPlugin.post_map_validate()`.
- **Runtime validator decoupling**: Removed `isinstance(config,
XYChartConfig)` hardcoding from `RuntimeValidator`; XY-specific
format/cardinality checks now live in `XYChartPlugin.get_runtime_warnings()`.
- **Stale docstring**: `generate_chart.py` docstring listed only `'xy'` and
`'table'` as valid chart types; updated to list all 7.
- **Pydantic error handling**: Added missing `pie`, `pivot_table`,
`mixed_timeseries` cases to `_enhance_validation_error`; refactored to a
data-driven lookup table to reduce cyclomatic complexity; fixed empty `details`
fallback.
**Architecture**: New files added:
- `superset/mcp_service/chart/plugin.py` — `ChartTypePlugin` Protocol +
`BaseChartPlugin` base class
- `superset/mcp_service/chart/registry.py` — central registry
-
`superset/mcp_service/chart/plugins/{xy,table,pie,pivot_table,mixed_timeseries,handlebars,big_number}.py`
— 7 plugin implementations
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend-only change, no UI modifications.
### TESTING INSTRUCTIONS
1. Start the MCP service: `python -m superset.mcp_service.server`
2. Test column validation for previously-unvalidated chart types:
- Create a pie chart with a non-existent dimension column → should get a
column-not-found error with suggestions
- Create a pivot_table with a non-existent metric column → same
- Previously these would silently pass validation and fail at query time
3. Test BigNumber trendline validation:
- Create a big_number chart with `show_trendline=true` but no
`temporal_column` → should error
- Create with a non-temporal column → should error with helpful message
4. Test generate_chart with all 7 chart types to verify no regressions
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
