Skip to content

Conversation

@aminghadersohi
Copy link
Contributor

Summary

[PROOF OF CONCEPT]

This PR implements FastMCP 2.13.0's new response caching middleware to solve scalability issues for the Superset MCP service. The caching layer dramatically improves performance for repeated queries and reduces database load.

Note: FastMCP 2.13.0 is already available in apache/superset master (pyproject.toml line 143). This PR builds on that upgrade to add caching capabilities.

Key Features

1. Response Caching Middleware

  • ✅ TTL-based caching for tools, resources, and prompts
  • ✅ In-memory cache by default (pluggable backends supported)
  • ✅ Configurable cache durations per operation type
  • ✅ Tool-level granularity for cache control

2. Smart Caching Defaults

List Operations (5 minute TTL):

  • list_tools, list_resources, list_prompts
  • list_dashboards, list_charts, list_datasets

Individual Items (1 hour TTL):

  • get_dashboard_info, get_chart_info, get_chart_preview
  • get_dataset_info, read_resource, get_prompt

Excluded from Cache (mutable/expensive operations):

  • execute_sql - SQL execution results vary
  • generate_dashboard, generate_chart - AI-generated content
  • update_chart, update_chart_preview - Data modification
  • add_chart_to_existing_dashboard - Data modification

3. Configuration

Users can customize caching behavior in superset_config.py:

MCP_CACHE_CONFIG = {
    "enabled": True,  # Enable/disable caching
    "max_item_size": 1024 * 1024,  # 1MB max cached item size
    
    # TTLs in seconds
    "list_tools_ttl": 300,      # 5 minutes
    "call_tool_ttl": 3600,      # 1 hour
    
    # Tool-specific control
    "excluded_tools": ["execute_sql", "generate_chart"],
    "included_tools": None,  # If set, only cache these tools
}

Changes

app.py

  • Removed deprecated stateless_http=True parameter (deprecated in FastMCP 2.13+)
  • Added middleware parameter to create_mcp_app()
  • Added middleware support to MCP instance factory
  • Updated default mcp instance creation to use server-level lifespan management

mcp_config.py

  • Added MCP_CACHE_CONFIG with sensible defaults
  • Implemented create_response_caching_middleware() factory
  • Supports configurable TTLs and tool filters
  • Graceful fallback for older FastMCP versions

server.py

  • Integrated caching middleware into server initialization
  • Middleware automatically enabled in default mode
  • Removed unused init_fastmcp_server import (ruff fix)
  • Created middleware list with response caching

Benefits

🚀 Performance:

  • Instant response for cached queries (no database hit)
  • Reduced latency for repeated list operations
  • Scales better with multiple concurrent clients

💾 Database Load:

  • Fewer queries to Superset database
  • Reduced load on metadata tables
  • More efficient resource utilization

⚙️ Flexibility:

  • Configurable per-operation TTLs
  • Tool-level caching control
  • Pluggable cache backends (memory, Redis, disk, etc.)

Testing

✅ MCP instance creates successfully with middleware
✅ ResponseCachingMiddleware attaches properly
✅ Configuration factory works correctly
✅ Pre-commit hooks pass (mypy, ruff, pylint)

Before/After

Without caching:

list_dashboards() → Database query every time
list_dashboards() → Database query every time
list_dashboards() → Database query every time

With caching:

list_dashboards() → Database query (cached for 5 min)
list_dashboards() → Cache hit (instant)
list_dashboards() → Cache hit (instant)

Migration Path

This is backward compatible - no changes required for existing deployments:

  • Caching is enabled by default with safe settings
  • Expensive/mutable operations are excluded
  • Users can disable via MCP_CACHE_CONFIG["enabled"] = False
  • Gracefully falls back if FastMCP 2.13+ not available

Next Steps

For production deployment, consider:

  1. Persistent cache backend - Replace in-memory cache with Redis/disk storage
  2. Cache statistics - Monitor hit rates via middleware.statistics()
  3. Cache invalidation - Implement webhook-based cache clearing on data changes
  4. TTL tuning - Adjust based on actual usage patterns

Related


Note: This is a proof of concept to demonstrate the caching capabilities. Feedback welcome!

@dosubot dosubot bot added change:backend Requires changing the backend dashboard:performance Related to Dashboard performance labels Nov 12, 2025
Copy link

@korbit-ai korbit-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.
Category Issue Status
Performance Inefficient middleware creation on startup ▹ view
Functionality Ambiguous middleware parameter handling ▹ view
Functionality Missing error handling for middleware creation ▹ view
Files scanned
File Path Reviewed
superset/mcp_service/server.py
superset/mcp_service/app.py
superset/mcp_service/mcp_config.py

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

Comment on lines 88 to 95
# Import here to avoid circular imports
from superset.mcp_service.mcp_config import create_response_caching_middleware

# Create middleware list - response caching is enabled by default
middleware_list = []
caching_middleware = create_response_caching_middleware()
if caching_middleware:
middleware_list.append(caching_middleware)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inefficient middleware creation on startup category Performance

Tell me more
What is the issue?

The caching middleware is created and imported on every server startup in the hot path, even when it might not be needed or when the server is restarted frequently.

Why this matters

This causes unnecessary overhead during server initialization and repeated imports/function calls that could be avoided through lazy loading or module-level caching.

Suggested change ∙ Feature Preview

Move the middleware creation to module level or implement lazy initialization. Consider caching the middleware instance or only creating it when actually needed:

# At module level
_cached_middleware = None

def get_cached_middleware():
    global _cached_middleware
    if _cached_middleware is None:
        from superset.mcp_service.mcp_config import create_response_caching_middleware
        _cached_middleware = create_response_caching_middleware()
    return _cached_middleware
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

middleware_list.append(caching_middleware)

# Create MCP instance with middleware
mcp_instance = create_mcp_app(middleware=middleware_list or None)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ambiguous middleware parameter handling category Functionality

Tell me more
What is the issue?

The middleware parameter is passed as None when middleware_list is empty, but this may not be the intended behavior for the create_mcp_app function.

Why this matters

If create_mcp_app expects an empty list instead of None for no middleware, this could cause unexpected behavior or errors. The function signature and expected parameter types are not visible, making this a potential runtime issue.

Suggested change ∙ Feature Preview

Either pass an empty list consistently or verify that create_mcp_app properly handles None values for middleware:

# Option 1: Always pass a list
mcp_instance = create_mcp_app(middleware=middleware_list)

# Option 2: Only pass middleware if not empty
if middleware_list:
    mcp_instance = create_mcp_app(middleware=middleware_list)
else:
    mcp_instance = create_mcp_app()
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

Comment on lines 93 to 95
caching_middleware = create_response_caching_middleware()
if caching_middleware:
middleware_list.append(caching_middleware)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing error handling for middleware creation category Functionality

Tell me more
What is the issue?

The code assumes create_response_caching_middleware() can return None or a falsy value, but there's no error handling if the function fails or raises an exception.

Why this matters

If create_response_caching_middleware() raises an exception, the server startup will fail. Additionally, if it returns an unexpected type that evaluates to False but isn't None, the middleware won't be added when it should be.

Suggested change ∙ Feature Preview

Add proper error handling and explicit None checking:

try:
    caching_middleware = create_response_caching_middleware()
    if caching_middleware is not None:
        middleware_list.append(caching_middleware)
except Exception as e:
    logging.warning("Failed to create response caching middleware: %s", e)
    # Continue without caching middleware
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

@aminghadersohi aminghadersohi force-pushed the feat/mcp-service-fastmcp-caching branch from d7fcffc to 7866f28 Compare November 12, 2025 02:41
Copy link
Contributor

@bito-code-review bito-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Agent Run #0b4305

Actionable Suggestions - 1
  • superset/mcp_service/server.py - 1
    • Broken MCP tool registration on new instance · Line 98-98
Additional Suggestions - 5
  • superset/mcp_service/mcp_config.py - 5
    • Use specific exception handling · Line 311-311
      Avoid catching blind `Exception`. Consider catching specific exceptions or use `logging.exception` instead of `logging.error`.
      Code suggestion
       @@ -311,2 +311,2 @@
      -    except Exception as e:
      -        logger.error("Failed to create response caching middleware: %s", e)
      +    except Exception as e:
      +        logger.exception("Failed to create response caching middleware: %s", e)
    • Use lowercase dict for type annotations · Line 226-226
      Use `dict` instead of `Dict` for type annotation. This applies to multiple locations (lines 226, 272, 316).
      Code suggestion
       @@ -226,1 +226,1 @@
      -    app_config: Dict[str, Any] | None = None,
      +    app_config: dict[str, Any] | None = None,
    • Fix multi-line docstring format · Line 228-228
      Multi-line docstring summary should start at the first line and needs blank line after last section.
      Code suggestion
       @@ -228,8 +228,9 @@
      -    """
      -    Create ResponseCachingMiddleware with configuration from app.config.
      +    """Create ResponseCachingMiddleware with configuration from app.config.
       
            Args:
                app_config: Optional Flask app configuration dict
       
            Returns:
                ResponseCachingMiddleware instance or None if caching is disabled
      +
            """
    • Add missing trailing comma · Line 301-301
      Trailing comma missing. This also applies to line 308.
      Code suggestion
       @@ -301,1 +301,1 @@
      -                "Excluded tools from caching: %s", cache_config["excluded_tools"]
      +                "Excluded tools from caching: %s", cache_config["excluded_tools"],
                    )
    • Move return to else block · Line 304-304
      Consider moving the return statement to an `else` block for better code structure.
      Code suggestion
       @@ -304,10 +304,11 @@
      -        return middleware
       
            except ImportError:
                logger.warning(
                    "ResponseCachingMiddleware not available - requires fastmcp 2.13+"
                )
                return None
      +    else:
      +        return middleware
Review Details
  • Files reviewed - 7 · Commit Range: b12bef3..d7fcffc
    • pyproject.toml
    • requirements/base.txt
    • requirements/development.txt
    • superset/mcp_service/app.py
    • superset/mcp_service/mcp_config.py
    • superset/mcp_service/server.py
    • tests/unit_tests/mcp_service/sql_lab/tool/test_execute_sql.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

middleware_list.append(caching_middleware)

# Create MCP instance with middleware
mcp_instance = create_mcp_app(middleware=middleware_list or None)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broken MCP tool registration on new instance

The change from init_fastmcp_server() to create_mcp_app(middleware=middleware_list or None) creates a new FastMCP instance that does not include the registered tools, since tools are registered on the global mcp instance in app.py. This breaks the MCP server's core functionality by making all tools unavailable. To fix this, use the global mcp instance and add the middleware to it using add_middleware(). The run_server function in server.py calls this initialization, and downstream the mcp_instance.run() method expects a properly configured FastMCP instance with tools.

Code suggestion
Check the AI-generated fix before applying
 -from superset.mcp_service.app import create_mcp_app
 +from superset.mcp_service.app import create_mcp_app, mcp
 @@ -98,1 +98,4 @@
 -        mcp_instance = create_mcp_app(middleware=middleware_list or None)
 +        mcp_instance = mcp
 +        for mw in middleware_list:
 +            mcp.add_middleware(mw)
Citations

Code Review Run #0b4305


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

@aminghadersohi aminghadersohi marked this pull request as draft November 12, 2025 02:44
@codecov
Copy link

codecov bot commented Nov 12, 2025

Codecov Report

❌ Patch coverage is 0% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.64%. Comparing base (d123249) to head (0b8b67e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
superset/mcp_service/mcp_config.py 0.00% 36 Missing ⚠️
superset/mcp_service/server.py 0.00% 12 Missing ⚠️
superset/mcp_service/app.py 0.00% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #36078       +/-   ##
===========================================
+ Coverage        0   68.64%   +68.64%     
===========================================
  Files           0      622      +622     
  Lines           0    45800    +45800     
  Branches        0     4985     +4985     
===========================================
+ Hits            0    31439    +31439     
- Misses          0    13115    +13115     
- Partials        0     1246     +1246     
Flag Coverage Δ
hive 44.13% <0.00%> (?)
mysql 67.72% <0.00%> (?)
postgres 67.77% <0.00%> (?)
presto 47.68% <0.00%> (?)
python 68.61% <0.00%> (?)
sqlite 67.38% <0.00%> (?)
unit 100.00% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This commit implements the new response caching middleware introduced
in FastMCP 2.13.0 to solve scalability issues for repeated/expensive
tool and resource operations.

**Key Changes:**

1. **Removed deprecated `stateless_http` parameter** (app.py:239)
   - Removed `stateless_http=True` from default MCP instance creation
   - Added comment explaining the deprecation in favor of server-level
     lifespan management and middleware-based caching

2. **Added middleware support to app factory** (app.py)
   - Added `middleware` parameter to `create_mcp_app()` and `_build_mcp_kwargs()`
   - Imported `Middleware` type from fastmcp.server.middleware
   - Updated function signatures to accept Sequence[Middleware]

3. **Implemented response caching configuration** (mcp_config.py)
   - Added `MCP_CACHE_CONFIG` with sensible defaults:
     - List operations: 5 minute TTL
     - Individual items: 1 hour TTL
     - Excluded tools: SQL execution and data modification operations
   - Created `create_response_caching_middleware()` factory function
   - Supports configurable TTLs, cache storage backends, and tool filters

4. **Integrated caching middleware into server** (server.py)
   - Modified `run_server()` to create and attach caching middleware
   - Middleware is enabled by default in non-factory-config mode
   - Graceful fallback if fastmcp 2.13+ is not available

**Benefits:**

- Dramatically improves performance for repeated queries
- Reduces database load for list operations (dashboards, charts, datasets)
- Configurable caching behavior per operation type
- Tool-level granularity for caching control
- In-memory cache by default (can be upgraded to persistent storage)

**Testing:**

- Verified MCP instance creation with middleware
- Confirmed ResponseCachingMiddleware is properly attached
- Tested configuration factory function

**Related:**

- Based on branch: chore/upgrade-fastmcp-to-latest
- FastMCP release: https://github.com/jlowin/fastmcp/releases/tag/v2.13.0

update

chore: revert unnecessary requirements file changes
- Use global mcp instance with add_middleware() instead of creating new instance (fixes tool registration)
- Add error handling for middleware creation with logging.exception
- Use Python 3.10+ lowercase dict type hints instead of Dict
- Fix multi-line docstring format with summary on first line
- Add missing trailing commas for better code style
- Improve middleware creation efficiency with try-except blocks
@aminghadersohi aminghadersohi force-pushed the feat/mcp-service-fastmcp-caching branch from 3387f70 to 0b8b67e Compare November 12, 2025 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:backend Requires changing the backend dashboard:performance Related to Dashboard performance size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant