sadpandajoe commented on code in PR #43842:
URL: https://github.com/apache/superset/pull/43842#discussion_r4418860648
##########
superset/core/mcp/core_mcp_injection.py:
##########
@@ -40,6 +41,59 @@
logger = logging.getLogger(__name__)
+# The concrete ``@tool``/``@prompt`` decorators register with the FastMCP
instance
+# that lives in ``superset.mcp_service.app``. That module is the memory-heavy
part
+# of MCP setup and is imported by ``initialize_core_mcp_host_tools`` before any
+# extension applies a decorator. Processes that never serve MCP (e.g. Celery
+# workers with ``CORE_MCP_HOST_TOOLS_ENABLED = False``) only swap the
decorators
+# and never import it. In those processes a decoration must stay a no-op rather
+# than importing the service app on demand -- otherwise applying ``@tool`` in
an
+# extension would still load the host-tool stack and defeat the memory savings.
+_MCP_SERVICE_APP_MODULE = "superset.mcp_service.app"
+
+
+def _mcp_host_tools_loaded() -> bool:
+ """Whether the MCP service app (and thus the host tools) is loaded here."""
+ return _MCP_SERVICE_APP_MODULE in sys.modules
+
+
+# Registrations already reported as skipped, so each one is logged only once.
+_skipped_registrations: set[str] = set()
+
+
+def _warn_skipped_registration(kind: str, base_id: str) -> None:
+ """
+ Log (once per registration) that an ``@tool``/``@prompt`` decoration was
+ skipped because the MCP service app is not loaded in this process.
+
+ This is the intended behavior in processes that never serve MCP, but if
+ ``CORE_MCP_HOST_TOOLS_ENABLED = False`` reaches a process that does serve
+ MCP, the registration would otherwise vanish without any trace.
+ """
+ registration_id, _ = _get_prefixed_id_with_context(base_id)
+ key = f"{kind}:{registration_id}"
+ if key in _skipped_registrations:
+ return
+ _skipped_registrations.add(key)
+ logger.warning(
+ "Skipping MCP %s registration for %s: the MCP service app is not
loaded "
+ "in this process (CORE_MCP_HOST_TOOLS_ENABLED is False). This is
expected "
Review Comment:
This warning states the flag is False, but the guard only checks whether
`superset.mcp_service.app` is in `sys.modules`. With the default
`CORE_MCP_HOST_TOOLS_ENABLED = True`, if `initialize_core_mcp_host_tools()`
hits its caught import error, the app never loads and each extension
`@tool`/`@prompt` then logs that the flag is False, sending an operator to
enable a setting that is already enabled. Could the message describe the
observed condition (service app not loaded) and mention the flag only as one
possible cause?
--
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]
