bito-code-review[bot] commented on PR #44004:
URL: https://github.com/apache/superset/pull/44004#issuecomment-5604367647
<!-- Bito Reply -->
The current implementation addresses the detachment issue by capturing the
necessary chart attributes into a `_ChartFacts` named tuple while the ORM
instance is still attached. This ensures that downstream helpers, which run
after session commits and await points, operate on stable, detached-safe values
rather than the potentially expired ORM instance.
To further harden the guest authorization path, you could consider
re-fetching the `Slice` object within the authorization scope if the initial
instance is detected as detached, or explicitly re-attaching it to the current
session. Extending the regression test to specifically trigger a detachment
scenario—such as by introducing an artificial `await` point or session commit
between the initial lookup and the authorization call—would verify that the
guest authorization and tamper guard logic correctly handle the detached state.
**superset/mcp_service/chart/tool/get_chart_data.py**
```
class _ChartFacts(NamedTuple):
"""Chart values copied off the Slice while it is still session-attached.
..."""
id: int
slice_name: str | None
viz_type: str | None
datasource_id: Any = None
datasource_type: str | None = None
```
--
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]
