aminghadersohi commented on PR #36038: URL: https://github.com/apache/superset/pull/36038#issuecomment-3505451237
## Addressed Review Feedback Fixed the hardcoded `stateless_http` parameter concern raised by @korbit-ai[bot]. ### Changes in commit 9b7f1b3: Made the `stateless_http` parameter configurable via the `FASTMCP_STATELESS_HTTP` environment variable while maintaining backward-compatible behavior (defaults to `true`). **Usage:** ```bash # Default behavior (stateless=true) superset mcp run # Disable stateless mode if needed export FASTMCP_STATELESS_HTTP=false superset mcp run ``` This addresses the concern about hardcoding the parameter and provides flexibility for users who may need stateful HTTP mode in specific scenarios, while keeping the recommended default behavior. ### Other Review Comments: **Incomplete alias handling** - This appears to be feedback for a different set of files (field filtering logic) that aren't part of this PR. This PR only modifies: - `superset/mcp_service/__main__.py` - `superset/mcp_service/app.py` - `superset/mcp_service/server.py` **Global mutable state** - This is an architectural design decision that's outside the scope of this deprecation fix. The module-level MCP instance follows FastMCP's decorator pattern (`@mcp.tool`) which requires a module-level instance for tool registration. --- The primary concern (hardcoded parameter) has been addressed. Ready for re-review! 🚀 -- 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]
