aminghadersohi commented on code in PR #39917:
URL: https://github.com/apache/superset/pull/39917#discussion_r3203145757
##########
superset/mcp_service/auth.py:
##########
@@ -516,6 +516,45 @@ def _cleanup_session_on_error() -> None:
logger.warning("Error cleaning up session after exception: %s", e)
+def _remove_session_safe() -> None:
+ """Remove the scoped SQLAlchemy session, tolerating SSL/connection errors.
+
+ Thread-pool workers reuse threads across requests. Before each tool call
+ the session is removed to prevent a prior request's thread-local session
+ from leaking into the next one. If the underlying DBAPI connection died
+ between requests (e.g. RDS SSL idle-timeout or max-connection-age), the
+ rollback implicit in ``session.close()`` raises ``OperationalError``.
+
+ When that happens:
+ 1. Invalidate the dead connection so the pool discards it (rather than
+ returning a broken connection to the next caller).
+ 2. Retry ``remove()`` to deregister the session from the scoped registry.
+
+ The tool call still proceeds because a fresh connection will be obtained
+ on the next DB access.
+ """
+ from sqlalchemy.exc import OperationalError
+
+ from superset.extensions import db
+
+ try:
+ db.session.remove()
+ except OperationalError as exc:
Review Comment:
Addressed — widened catch from `OperationalError` to `DBAPIError` in
`_remove_session_safe()` so drivers that surface disconnects as
`InterfaceError` (or other `DBAPIError` subclasses) are also handled. — agor
claude on Amin's behalf
--
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]
