feat(mcp): allow skipping MCP host-tool registration in non-MCP processes - #43840
Closed
eschutho wants to merge 1 commit into
Closed
feat(mcp): allow skipping MCP host-tool registration in non-MCP processes#43840eschutho wants to merge 1 commit into
eschutho wants to merge 1 commit into
Conversation
eschutho
requested review from
michael-s-molina,
rusackas,
sadpandajoe and
villebro
as code owners
September 4, 2026 00:15
Contributor
Code Review Agent Run #bf55fbActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| # - deprecated: Will be removed in a future major release | ||
|
|
||
| DEFAULT_FEATURE_FLAGS: dict[str, bool] = { | ||
| DEFAULT_# Whether this process registers the MCP host tools at startup. Only processes |
Contributor
There was a problem hiding this comment.
Suggestion: The bare DEFAULT_ expression is evaluated while importing the default configuration, raising NameError and preventing every Superset process from starting. [import error]
Assessment: 🔴 Critical · 🔁 Occurrence: Often
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/config.py
**Line:** 542:542
**Comment:**
*Import Error: The bare `DEFAULT_` expression is evaluated while importing the default configuration, raising `NameError` and preventing every Superset process from starting.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| # (the web app and the standalone MCP service). Deployments can disable | ||
| # this in processes that never serve MCP -- e.g. Celery workers -- to | ||
| # avoid importing the MCP stack where it is unused. | ||
| if self.config.get("CORE_MCP_HOST_TOOLS_ENABLED", True): |
Contributor
There was a problem hiding this comment.
Suggestion: When this flag is false, MCP decorators remain abstract and raise NotImplementedError, so extensions defining MCP tools or prompts fail during eager import and are skipped. [incomplete implementation]
Assessment: 🟠 Major · 🔁 Occurrence: Sometimes
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset/initialization/__init__.py
**Line:** 565:565
**Comment:**
*Incomplete Implementation: When this flag is false, MCP decorators remain abstract and raise `NotImplementedError`, so extensions defining MCP tools or prompts fail during eager import and are skipped.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
…sses init_core_dependencies() calls initialize_core_mcp_dependencies() in every process that builds the app, including Celery workers that never serve MCP. Importing the MCP service app to register host tools has a real per-process memory cost that can OOM high-concurrency prefork worker pools on boot. Add a CORE_MCP_HOST_TOOLS_ENABLED config flag (default True, no behavior change) and gate the call on it, so deployments can turn host-tool registration off for processes that do not serve MCP. Follows the make-MCP-optional-at-startup direction of apache#38534. Signed-off-by: Elizabeth Thompson <elizabeth@preset.io> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
eschutho
force-pushed
the
fix/gate-mcp-host-tools-off-workers
branch
from
September 4, 2026 00:21
abe11b3 to
40f04c3
Compare
eschutho
requested review from
Antonio-RiveroMartnez,
betodealmeida,
dpgaspar,
geido,
hainenber,
kgabryje,
mistercrunch,
nytai and
sha174n
as code owners
September 4, 2026 00:21
github-actions
Bot
added
doc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SUMMARY
SupersetAppInitializer.init_core_dependencies()callsinitialize_core_mcp_dependencies()in every process that builds the app — including Celery workers, which never serve MCP. That function imports the MCP service app to register all host tools, which carries a real per-process memory cost. In high-concurrency prefork worker pools, that can OOM the pool on boot.This adds a
CORE_MCP_HOST_TOOLS_ENABLEDconfig flag (defaultTrue, so no behavior change) and gates the call on it, so a deployment can disable host-tool registration in processes that don't serve MCP (e.g. Celery workers). This follows the "make MCP optional at startup" direction of #38534.TESTING INSTRUCTIONS
Set
CORE_MCP_HOST_TOOLS_ENABLED = Falseinsuperset_config.py; confirm the app starts and theMCP service app imported - host tools registeredlog no longer appears. With the default (True), behavior is unchanged and MCP-serving processes work as before.ADDITIONAL INFORMATION
True)Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com