Skip to content

fix(mcp): add try/except around DAO re-fetch to handle session errors in multi-tenant#38859

Merged
aminghadersohi merged 3 commits into
apache:masterfrom
aminghadersohi:amin/fix-mcp-session-refetch
Mar 26, 2026
Merged

fix(mcp): add try/except around DAO re-fetch to handle session errors in multi-tenant#38859
aminghadersohi merged 3 commits into
apache:masterfrom
aminghadersohi:amin/fix-mcp-session-refetch

Conversation

@aminghadersohi
Copy link
Copy Markdown
Contributor

@aminghadersohi aminghadersohi commented Mar 25, 2026

User description

Summary

  • Wraps the post-commit DashboardDAO.find_by_id() / ChartDAO.find_by_id() re-fetch calls in try/except SQLAlchemyError with db.session.rollback() fallback in generate_dashboard, add_chart_to_existing_dashboard, and generate_chart
  • In multi-tenant environments the session can become invalid after a commit, causing "Can't reconnect until invalid transaction is rolled back" — the try/except gracefully falls back to the original object
  • Narrows broad except Exception to specific exceptions (CommandException, SQLAlchemyError, KeyError, ValueError) in generate_chart and add_chart_to_existing_dashboard
  • Adds logger.warning() with exc_info=True in re-fetch failure paths for operational visibility
  • Filters None values from serialize_chart_object in add_chart_to_existing_dashboard chart list

Test plan

  • All 955 MCP unit tests pass
  • Tested generate_dashboard on staging — created dashboard successfully
  • Tested add_chart_to_existing_dashboard on staging — added chart successfully
  • Test generate_chart on a workspace with working examples DB

CodeAnt-AI Description

Keep dashboard and chart creation working after commit in multi-tenant sessions

What Changed

  • Dashboard creation, chart generation, and adding a chart to a dashboard now fall back cleanly if the post-save refresh cannot be loaded because the session was invalidated
  • When that refresh fails, the tool rolls back the session and still returns the created or updated dashboard/chart instead of failing the request
  • Adding a chart now skips empty chart entries in the returned dashboard details
  • Tests now cover successful refresh, fallback behavior, and session rollback for these flows

Impact

✅ Fewer dashboard creation failures
✅ Fewer chart save errors after commit
✅ Fewer broken add-chart actions in multi-tenant workspaces

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Mar 25, 2026

Code Review Agent Run #6185a2

Actionable Suggestions - 0
Additional Suggestions - 1
  • tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py - 1
    • Incomplete test assertions · Line 413-440
      The updated tests verify the fallback logic but do not assert that the db.session.query chain is invoked with the expected joinedload options and filter condition, unlike the previous DAO-based tests that checked the call. This reduces test robustness and may allow bugs in the refetch behavior to go undetected.
Review Details
  • Files reviewed - 5 · Commit Range: 05e3d1d..05e3d1d
    • superset/mcp_service/chart/tool/generate_chart.py
    • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
    • superset/mcp_service/dashboard/tool/generate_dashboard.py
    • tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.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 Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot Bot added the change:backend Requires changing the backend label Mar 25, 2026
@codeant-ai-for-open-source codeant-ai-for-open-source Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Mar 25, 2026
@codeant-ai-for-open-source
Copy link
Copy Markdown
Contributor

Sequence Diagram

This PR updates MCP save flows to re-load dashboards and charts using the active db session instead of DAO lookups, preventing stale session failures after commit in multi-tenant setups. It also limits top-level error handling to expected exception types for clearer failure responses.

sequenceDiagram
    participant Client
    participant MCP Tool
    participant Command
    participant DB Session
    participant Serializer

    Client->>MCP Tool: Request to generate or update dashboard or chart
    MCP Tool->>Command: Save or update entity
    Command-->>MCP Tool: Return saved entity
    MCP Tool->>DB Session: Requery same entity with eager relations

    alt Requery succeeds
        DB Session-->>MCP Tool: Session bound entity
    else Requery fails
        DB Session-->>MCP Tool: Database error
        MCP Tool->>MCP Tool: Fallback to saved entity
    end

    MCP Tool->>Serializer: Serialize entity for response
    MCP Tool-->>Client: Return success or specific handled error
Loading

Generated by CodeAnt AI

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 25, 2026

Codecov Report

❌ Patch coverage is 3.03030% with 32 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.42%. Comparing base (23a5e95) to head (c4da7d6).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
superset/mcp_service/chart/tool/generate_chart.py 6.66% 14 Missing ⚠️
.../dashboard/tool/add_chart_to_existing_dashboard.py 0.00% 11 Missing ⚠️
...t/mcp_service/dashboard/tool/generate_dashboard.py 0.00% 7 Missing ⚠️

❌ Your project status has failed because the head coverage (99.85%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #38859      +/-   ##
==========================================
- Coverage   65.54%   64.42%   -1.13%     
==========================================
  Files        1823     2536     +713     
  Lines       73154   130719   +57565     
  Branches    23437    30308    +6871     
==========================================
+ Hits        47951    84211   +36260     
- Misses      25203    45042   +19839     
- Partials        0     1466    +1466     
Flag Coverage Δ
hive 40.36% <3.03%> (?)
mysql 61.28% <3.03%> (?)
postgres 61.36% <3.03%> (?)
presto 40.37% <3.03%> (?)
python 62.96% <3.03%> (?)
sqlite 60.98% <3.03%> (?)
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.

Comment thread superset/mcp_service/chart/tool/generate_chart.py Outdated
Comment thread superset/mcp_service/dashboard/tool/generate_dashboard.py Outdated
Comment thread tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py Outdated
Comment thread tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py Outdated
Comment thread tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py Outdated
Comment thread tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py Outdated
Comment thread tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py Outdated
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Mar 25, 2026

Code Review Agent Run #fb4952

Actionable Suggestions - 0
Review Details
  • Files reviewed - 5 · Commit Range: 05e3d1d..c6d7892
    • superset/mcp_service/chart/tool/generate_chart.py
    • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
    • superset/mcp_service/dashboard/tool/generate_dashboard.py
    • tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.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 Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@aminghadersohi aminghadersohi changed the title fix(mcp): replace DAO re-fetch with db.session.query() to fix session errors fix(mcp): add try/except around DAO re-fetch to handle session errors in multi-tenant Mar 26, 2026
@codeant-ai-for-open-source codeant-ai-for-open-source Bot added size:L This PR changes 100-499 lines, ignoring generated files and removed size:L This PR changes 100-499 lines, ignoring generated files labels Mar 26, 2026
Comment thread tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py
Comment thread superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py Outdated
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Mar 26, 2026

Code Review Agent Run #a90f89

Actionable Suggestions - 0
Review Details
  • Files reviewed - 5 · Commit Range: c6d7892..0cd9774
    • superset/mcp_service/chart/tool/generate_chart.py
    • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
    • superset/mcp_service/dashboard/tool/generate_dashboard.py
    • tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.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 Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

…ommit

In multi-tenant environments, the DAO re-fetch after commit can fail
with "Can't reconnect until invalid transaction is rolled back". The
previous fallback to the "original" object was unsound — the original
is bound to the same dead session, so accessing lazy-loaded
relationships (tags, owners, slices) would trigger DetachedInstanceError.

Fix: follow the update_chart.py pattern — when the DAO re-fetch fails,
build a minimal response from scalar attributes that are already loaded
on the object (id, title, url, etc.), skipping relationship fields
entirely. This avoids any further session access.

Changes:
- generate_dashboard: return minimal DashboardInfo on re-fetch failure
- add_chart_to_existing_dashboard: same, plus position info
- generate_chart: build minimal chart_data dict, skip serialize_chart_object
- Move SQLAlchemyError/CommandException imports before try blocks
- Narrow outer except clauses to specific exception types
- Update tests to assert minimal response content (id, url, empty relationships)
@aminghadersohi aminghadersohi force-pushed the amin/fix-mcp-session-refetch branch from 0cd9774 to 172131c Compare March 26, 2026 10:36
@aminghadersohi
Copy link
Copy Markdown
Contributor Author

Addressing review comments — all squashed into single commit 172131c:

Re: codeant bot concern about _make_mock_chart() and type(chart).tags = property(...):
This is incorrect — Mock.__setattr__ bypasses the descriptor protocol, so setting a property on type(chart) does not prevent instance attribute assignment on other Mock instances. All 955 MCP tests pass.

Re: "i am looking into a better solution" (stale session fallback):
Resolved. On re-fetch failure, we now return a minimal response using only scalar attributes (id, title, url) that are already loaded on the object — no lazy-loaded relationships (owners, tags, slices) are accessed. This follows the update_chart.py pattern (lines 238-270) which builds its response from scalar attributes without any re-fetch.

All prior review comments (rollback, import ordering, test coverage) were addressed in earlier commits and are now squashed into this single commit.

Remove parentheses from @pytest.mark.asyncio() and @pytest.fixture()
decorators to comply with ruff 0.9.7 PT023/PT001 rules.
Copy link
Copy Markdown
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 #657907

Actionable Suggestions - 1
  • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py - 1
Review Details
  • Files reviewed - 5 · Commit Range: 172131c..c4da7d6
    • superset/mcp_service/chart/tool/generate_chart.py
    • superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py
    • superset/mcp_service/dashboard/tool/generate_dashboard.py
    • tests/unit_tests/mcp_service/chart/tool/test_generate_chart.py
    • tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.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 Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@aminghadersohi aminghadersohi merged commit 6dc3d7a into apache:master Mar 26, 2026
64 of 65 checks passed
michael-s-molina pushed a commit that referenced this pull request Mar 30, 2026
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
@github-actions github-actions Bot added 🍒 6.1.0 Cherry-picked to 6.1.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels change:backend Requires changing the backend size/L size:L This PR changes 100-499 lines, ignoring generated files 🍒 6.1.0 Cherry-picked to 6.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants