DEV-1259: MCP dep fix#32
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughMade the MCP package a required dependency (removed optional extra), updated documentation and examples to drop the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
slayer/mcp/server.py (1)
115-116: Preserve the original import failure context by chaining the exception.The ImportError raised at line 116 drops the original exception cause, which loses diagnostic information. Chain it with
from excso the traceback retains the actual import error.Proposed fix
- except ImportError: - raise ImportError("MCP package not found. Reinstall SLayer: pip install motley-slayer") + except ImportError as exc: + raise ImportError( + "MCP package not found. Reinstall SLayer: pip install motley-slayer" + ) from exc🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@slayer/mcp/server.py` around lines 115 - 116, The except ImportError block in slayer/mcp/server.py currently swallows the original import error; update the handler to capture the exception (use "except ImportError as exc") and re-raise the new ImportError with chaining ("raise ImportError('MCP package not found. Reinstall SLayer: pip install motley-slayer') from exc") so the original traceback is preserved when importing in the MCP initialization code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/getting-started/mcp.md`:
- Line 16: Several MCP documentation references still include the extra "mcp" in
the package extras list; replace every occurrence of the string
"motley-slayer[mcp,postgres]" with "motley-slayer[postgres]" in the MCP
documentation files (the getting-started and reference MCP docs). Search for the
exact token "motley-slayer[mcp,postgres]" and update it to
"motley-slayer[postgres]" in each occurrence so the extras list no longer
contains "mcp".
In `@docs/reference/mcp.md`:
- Line 28: Replace the incorrect extras specification
"motley-slayer[mcp,postgres]" with "motley-slayer[postgres]" in the command
examples so MCP isn't redundantly listed (update every occurrence of the literal
string "motley-slayer[mcp,postgres]" to "motley-slayer[postgres]" in the docs
reference for MCP); ensure the three command snippets that currently include
"mcp" in the extras are updated and leave other text unchanged.
---
Nitpick comments:
In `@slayer/mcp/server.py`:
- Around line 115-116: The except ImportError block in slayer/mcp/server.py
currently swallows the original import error; update the handler to capture the
exception (use "except ImportError as exc") and re-raise the new ImportError
with chaining ("raise ImportError('MCP package not found. Reinstall SLayer: pip
install motley-slayer') from exc") so the original traceback is preserved when
importing in the MCP initialization code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 68434329-b710-454f-be1d-a5482c415524
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
docs/getting-started/mcp.mddocs/reference/mcp.mdpyproject.tomlslayer/api/server.pyslayer/mcp/server.py
AivanF
force-pushed
the
aivan/dev-1259-slayer-quickstart-MCP-dep-issue
branch
from
bf352f5 to
4056411
Compare
6 tasks
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 by CodeRabbit
Documentation
Chores