aminghadersohi commented on code in PR #39922:
URL: https://github.com/apache/superset/pull/39922#discussion_r3328062991
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -390,6 +390,24 @@ async def update_chart( # noqa: C901
# config is already a typed ChartConfig | None (validated by Pydantic)
parsed_config = request.config
+ # Normalize column case to match dataset canonical names
+ # (mirrors generate_chart pipeline layer 4)
+ chart_datasource_id = getattr(chart, "datasource_id", None)
+ if parsed_config is not None and chart_datasource_id is not None:
+ from superset.mcp_service.chart.validation.dataset_validator
import (
+ DatasetValidator,
+ NORMALIZATION_EXCEPTIONS,
+ )
+
+ try:
+ parsed_config = DatasetValidator.normalize_column_names(
+ parsed_config, chart.datasource_id
+ )
Review Comment:
The normalization call already uses the local variable. Current
`update_chart.py` lines 432–435:
```python
try:
parsed_config = DatasetValidator.normalize_column_names(
parsed_config, chart_datasource_id # ← local var, not
chart.datasource_id
)
```
`chart_datasource_id` is the result of `getattr(chart, 'datasource_id',
None)` from line 425 and is what is passed to `normalize_column_names`. The
other `chart.datasource_id` references in the file are in separate helper
functions (`_build_update_payload`, `_create_preview_url`) with their own scope.
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -390,6 +390,24 @@ async def update_chart( # noqa: C901
# config is already a typed ChartConfig | None (validated by Pydantic)
parsed_config = request.config
+ # Normalize column case to match dataset canonical names
+ # (mirrors generate_chart pipeline layer 4)
+ chart_datasource_id = getattr(chart, "datasource_id", None)
+ if parsed_config is not None and chart_datasource_id is not None:
+ from superset.mcp_service.chart.validation.dataset_validator
import (
+ DatasetValidator,
+ NORMALIZATION_EXCEPTIONS,
+ )
+
+ try:
+ parsed_config = DatasetValidator.normalize_column_names(
+ parsed_config, chart.datasource_id
+ )
Review Comment:
The normalization call already uses the local variable. Current
`update_chart.py` lines 432–435:
```python
try:
parsed_config = DatasetValidator.normalize_column_names(
parsed_config, chart_datasource_id # ← local var, not
chart.datasource_id
)
```
`chart_datasource_id` is the result of `getattr(chart, 'datasource_id',
None)` from line 425 and is what is passed to `normalize_column_names`. The
other `chart.datasource_id` references in the file are in separate helper
functions (`_build_update_payload`, `_create_preview_url`) with their own scope.
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -390,6 +390,24 @@ async def update_chart( # noqa: C901
# config is already a typed ChartConfig | None (validated by Pydantic)
parsed_config = request.config
+ # Normalize column case to match dataset canonical names
+ # (mirrors generate_chart pipeline layer 4)
+ chart_datasource_id = getattr(chart, "datasource_id", None)
+ if parsed_config is not None and chart_datasource_id is not None:
+ from superset.mcp_service.chart.validation.dataset_validator
import (
+ DatasetValidator,
+ NORMALIZATION_EXCEPTIONS,
+ )
+
+ try:
+ parsed_config = DatasetValidator.normalize_column_names(
+ parsed_config, chart.datasource_id
+ )
Review Comment:
The normalization call already uses the local variable. Current
`update_chart.py` lines 432–435:
```python
try:
parsed_config = DatasetValidator.normalize_column_names(
parsed_config, chart_datasource_id # ← local var, not
chart.datasource_id
)
```
`chart_datasource_id` is the result of `getattr(chart, 'datasource_id',
None)` from line 425 and is what is passed to `normalize_column_names`. The
other `chart.datasource_id` references in the file are in separate helper
functions (`_build_update_payload`, `_create_preview_url`) with their own scope.
--
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]
