bito-code-review[bot] commented on code in PR #44581:
URL: https://github.com/apache/superset/pull/44581#discussion_r4113937151
##########
superset/mcp_service/middleware.py:
##########
@@ -218,6 +221,16 @@ def _invoke_error_hook(error: Exception, hook_context:
dict[str, Any]) -> None:
logger.warning("MCP_ERROR_HOOK raised an exception: %s", hook_error)
+async def _invoke_error_hook_off_loop(
+ error: Exception, hook_context: dict[str, Any]
+) -> None:
+ """Keep hook I/O and context-setup failures outside the error boundary."""
+ try:
+ await run_in_metadata_thread(_invoke_error_hook, error, hook_context)
+ except Exception as hook_error: # noqa: BLE001
+ logger.warning("Could not run MCP_ERROR_HOOK: %s", hook_error)
Review Comment:
<!-- Bito Reply -->
The suggestion to route the error hook through a metadata thread is
appropriate and addresses the identified database dependency issue. By moving
the hook execution outside the primary transport loop, the implementation
ensures that hook I/O and context-setup failures do not interfere with the main
error boundary, while also preventing unnecessary user-reload queries when no
hook is configured.
**superset/mcp_service/middleware.py**
```
+async def _invoke_error_hook_off_loop(
+ error: Exception, hook_context: dict[str, Any]
+) -> None:
+ """Keep hook I/O and context-setup failures outside the error
boundary."""
+ try:
+ await run_in_metadata_thread(_invoke_error_hook, error,
hook_context)
+ except Exception as hook_error: # noqa: BLE001
+ logger.warning("Could not run MCP_ERROR_HOOK: %s", hook_error)
```
--
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]
