sadpandajoe commented on code in PR #42621:
URL: https://github.com/apache/superset/pull/42621#discussion_r3685939265
##########
superset/mcp_service/chart/tool/generate_chart.py:
##########
@@ -424,14 +427,25 @@ async def generate_chart( # noqa: C901
)
chart = command.run()
- chart_id = chart.id
# Ensure chart was created successfully before committing
if not chart or not chart.id:
raise RuntimeError(
"Chart creation failed - no chart ID returned"
)
+ # Snapshot the scalar fields now, while the instance is
+ # known to be attached. The chart is already committed at
+ # this point, and every read further down happens after an
+ # await: under concurrency another in-flight request can
+ # tear down the shared session in between, which detaches
+ # this instance and turns any attribute access into a
+ # DetachedInstanceError.
+ chart_id = chart.id
+ chart_slice_name = chart.slice_name
+ chart_viz_type = chart.viz_type
+ chart_uuid = str(chart.uuid) if chart.uuid else None
Review Comment:
The next post-await step still reads `chart.datasource_id` through
`validate_chart_dataset`, so a concurrent commit and session teardown can
detach an expired instance here and report the already-committed chart as
failed. Could we snapshot/pass the datasource identity too and let the
regression test exercise this validator instead of mocking it?
--
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]
