aminghadersohi commented on code in PR #43679:
URL: https://github.com/apache/superset/pull/43679#discussion_r3944991984
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -230,12 +230,16 @@ def _build_update_payload(
)
if parsed_config is not None:
+ existing_form_data = _get_existing_form_data(chart)
Review Comment:
Thanks for flagging this. This does not introduce a new concurrency window:
the immediate-update flow already reads the chart and replaces the complete
`params` value through `UpdateChartCommand`, without an expected-version field
or optimistic-lock contract. Merging the saved form data changes which keys are
included in that same replacement, but concurrent Explore saves could already
race with it. Adding versioning would require an API-wide concurrency design
and is outside this focused consistency fix.
##########
superset/mcp_service/chart/tool/update_chart.py:
##########
@@ -230,12 +230,16 @@ def _build_update_payload(
)
if parsed_config is not None:
+ existing_form_data = _get_existing_form_data(chart)
new_form_data = map_config_to_form_data(
parsed_config, dataset_id=effective_dataset_id
)
new_form_data.pop("_mcp_warnings", None)
- merge_table_column_config(_get_existing_form_data(chart),
new_form_data)
- merge_interactive_pivot_ui_config(_get_existing_form_data(chart),
new_form_data)
+ merge_table_column_config(existing_form_data, new_form_data)
+ merge_interactive_pivot_ui_config(existing_form_data, new_form_data)
+ new_form_data = _merge_replacement_config(
+ existing_form_data, new_form_data, parsed_config
+ )
Review Comment:
Fixed. Dataset-plus-config replacements now stamp `params.datasource` from
the replacement dataset ID after the merge, keeping it consistent with the
payload `datasource_id`. The regression test seeds `10__table` and asserts the
saved params contain `99__table`.
--
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]
