verdier commented on issue #42567:
URL: https://github.com/apache/superset/issues/42567#issuecomment-5117858104
Small but important correction: **all three of those PRs are already in
6.1.0**, so this reproduction happened *with* those fixes in place.
```
$ git merge-base --is-ancestor <commit> 6.1.0
PR #38767 -> 9239db5a32 : in 6.1.0
PR #38859 -> 8d47bd7f42 : in 6.1.0
PR #38827 -> 861ce50473 : in 6.1.0
```
And they are visible in the tag:
`superset/mcp_service/chart/tool/generate_chart.py` at 6.1.0 has the guarded
re-fetch at **lines 735-770** — `ChartDAO.find_by_id(...,
joinedload(Slice.owners), joinedload(Slice.tags))` inside `try/except
SQLAlchemyError` with a rollback.
The problem is that the guard does not cover the access that actually fails.
The traceback points at **line 828**, roughly 55 lines *after* that block, in
the completion log:
```python
await ctx.info(
"Chart generation completed successfully: chart_id=%s,
execution_time_ms=%s"
% (chart.id if chart else None, ...) # <- line 828, unguarded
)
```
By then, under concurrency, `chart` has been detached again — another
in-flight task's session teardown expires it — and `chart.id` triggers a lazy
refresh on a detached instance:
```
sqlalchemy.orm.exc.DetachedInstanceError: Instance <Slice at 0x...> is not
bound to a Session;
attribute refresh operation cannot proceed
File ".../generate_chart.py", line 828, in generate_chart
```
So #38767/#38859 hardened the serialisation path, and this is a second,
later access on the same object that they do not reach. The re-fetched chart is
not re-read at line 828 either — it reads the original reference.
That would also explain why the failure rate scales with concurrency rather
than being deterministic: it depends on whether another task tears the session
down between the guarded block and the log line.
If it helps, capturing `chart_id` as a plain int before the `try` and using
that in the completion log would sidestep it without another DB round-trip.
Happy to test any patch against the same 1/2/5/10/20 harness.
--
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]
