Fix: disable mcp sse endpoints astra#11006
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change introduces Astra Cloud environment guards across multiple MCP API endpoints. A new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 4 warnings)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (39.27%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11006 +/- ##
==========================================
+ Coverage 33.08% 33.09% +0.01%
==========================================
Files 1389 1389
Lines 65643 65646 +3
Branches 9707 9707
==========================================
+ Hits 21715 21723 +8
+ Misses 42816 42811 -5
Partials 1112 1112
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/base/langflow/api/v1/mcp_projects.py (1)
419-469: Apply Astra Cloud dependency guard to Streamable HTTP endpoints for consistency.The SSE transport endpoints are disabled in Astra Cloud environments via
raise_error_if_astra_cloud_envdependency, but the Streamable HTTP endpoints at lines 419-469 lack this guard. Since both transports serve identical MCP functionality, the same guard should be applied to both. Adddependencies=[Depends(raise_error_if_astra_cloud_env)]to the@router.head()and@router.api_route()decorators for consistency with the SSE pattern at lines 313-326 and the global Streamable HTTP endpoints.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/backend/base/langflow/api/utils/__init__.py(2 hunks)src/backend/base/langflow/api/utils/core.py(3 hunks)src/backend/base/langflow/api/v1/mcp.py(3 hunks)src/backend/base/langflow/api/v1/mcp_projects.py(3 hunks)src/lfx/src/lfx/utils/validate_cloud.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
src/backend/**/*.py: Use FastAPI async patterns withawaitfor async operations in component execution methods
Useasyncio.create_task()for background tasks and implement proper cleanup with try/except forasyncio.CancelledError
Usequeue.put_nowait()for non-blocking queue operations andasyncio.wait_for()with timeouts for controlled get operations
Files:
src/backend/base/langflow/api/utils/__init__.pysrc/backend/base/langflow/api/v1/mcp_projects.pysrc/backend/base/langflow/api/utils/core.pysrc/backend/base/langflow/api/v1/mcp.py
src/backend/base/langflow/api/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/backend_development.mdc)
Backend API endpoints should be organized by version (v1/, v2/) under
src/backend/base/langflow/api/with specific modules for features (chat.py, flows.py, users.py, etc.)
Files:
src/backend/base/langflow/api/utils/__init__.pysrc/backend/base/langflow/api/v1/mcp_projects.pysrc/backend/base/langflow/api/utils/core.pysrc/backend/base/langflow/api/v1/mcp.py
🧬 Code graph analysis (4)
src/backend/base/langflow/api/utils/__init__.py (1)
src/backend/base/langflow/api/utils/core.py (1)
raise_error_if_astra_cloud_env(423-425)
src/backend/base/langflow/api/v1/mcp_projects.py (2)
src/backend/base/langflow/api/utils/core.py (2)
extract_global_variables_from_headers(394-420)raise_error_if_astra_cloud_env(423-425)src/backend/base/langflow/api/v1/mcp.py (1)
im_alive(94-95)
src/backend/base/langflow/api/utils/core.py (1)
src/lfx/src/lfx/utils/validate_cloud.py (1)
raise_error_if_astra_cloud_disable_component(10-24)
src/backend/base/langflow/api/v1/mcp.py (1)
src/backend/base/langflow/api/utils/core.py (1)
raise_error_if_astra_cloud_env(423-425)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Run Backend Tests / Integration Tests - Python 3.10
- GitHub Check: Lint Backend / Run Mypy (3.11)
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Lint Backend / Run Mypy (3.10)
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
- GitHub Check: Lint Backend / Run Mypy (3.12)
- GitHub Check: Run Backend Tests / LFX Tests - Python 3.10
- GitHub Check: Test Starter Templates
- GitHub Check: Optimize new Python code in this PR
- GitHub Check: Update Starter Projects
🔇 Additional comments (8)
src/lfx/src/lfx/utils/validate_cloud.py (1)
23-23: LGTM: Cleaner conditional check.The refactor removes the unnecessary intermediate variable assignment, making the code more concise while maintaining the same behavior.
src/backend/base/langflow/api/utils/__init__.py (1)
36-36: LGTM: Public API export.The function is correctly re-exported to make it available through the utils module's public interface.
Also applies to: 74-74
src/backend/base/langflow/api/utils/core.py (1)
44-47: LGTM: Clear error message and simple delegation.The constant provides a clear message, and the function correctly delegates to the validation utility.
Also applies to: 423-425
src/backend/base/langflow/api/v1/mcp_projects.py (2)
34-38: LGTM: Imports correctly updated.The necessary imports are added to support the new Astra Cloud environment dependency.
312-317: LGTM: SSE endpoints correctly guarded.The dependency is correctly applied to all SSE transport endpoints (HEAD, GET, and POST routes) to disable them in Astra Cloud environments.
Also applies to: 322-326, 403-404
src/backend/base/langflow/api/v1/mcp.py (3)
5-5: LGTM: Imports correctly updated.The necessary imports (
Dependsandraise_error_if_astra_cloud_env) are added to support the FastAPI dependency pattern.Also applies to: 13-13
88-93: LGTM: SSE endpoints correctly guarded.The Astra Cloud environment dependency is correctly applied to all SSE transport endpoints using FastAPI's
dependenciesparameter.Also applies to: 98-102, 147-147
258-268: Verify: Should Streamable HTTP endpoints also be disabled?Similar to the project-specific endpoints, the global Streamable HTTP endpoints (
/streamableat lines 258-268) are not guarded with the Astra Cloud dependency, while SSE endpoints are.Confirm that this is intentional and that Streamable HTTP transport should remain available in Astra Cloud environments even when SSE is disabled.
| from lfx.graph.graph.base import Graph | ||
| from lfx.log.logger import logger | ||
| from lfx.services.deps import injectable_session_scope, injectable_session_scope_readonly, session_scope | ||
| from lfx.utils.validate_cloud import raise_error_if_astra_cloud_disable_component |
There was a problem hiding this comment.
Verify HTTP error response handling.
The imported raise_error_if_astra_cloud_disable_component raises a ValueError. When used as a FastAPI dependency, uncaught exceptions result in a 500 Internal Server Error response. For a feature availability check in a cloud environment, a more appropriate status code would be 403 Forbidden or 503 Service Unavailable.
Consider wrapping the call in a try-except block to catch ValueError and raise HTTPException with an appropriate status code instead.
def raise_error_if_astra_cloud_env():
"""Raise an error if we're in an Astra cloud environment."""
- raise_error_if_astra_cloud_disable_component(disable_endpoint_in_astra_cloud_msg)
+ try:
+ raise_error_if_astra_cloud_disable_component(disable_endpoint_in_astra_cloud_msg)
+ except ValueError as e:
+ from fastapi import HTTPException
+ raise HTTPException(status_code=403, detail=str(e))Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/backend/base/langflow/api/utils/core.py around line 14, the dependency
call to raise_error_if_astra_cloud_disable_component currently lets ValueError
bubble up causing a 500; catch ValueError and convert it to a FastAPI
HTTPException with an appropriate status (use status.HTTP_403_FORBIDDEN or
status.HTTP_503_SERVICE_UNAVAILABLE) and a clear detail message. Update the
function to import HTTPException and status from fastapi, wrap the call in
try/except ValueError, and in the except raise
HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail=str(err)) (or 503 if
you prefer service-unavailable semantics) so the API returns the correct HTTP
response instead of 500.
There was a problem hiding this comment.
note to self: this is good and should be a follow up task
There was a problem hiding this comment.
@HzaRashid, sounds good! Would you like me to create a GitHub issue to track this follow-up task for improving the HTTP error response handling in the MCP endpoints?
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
HzaRashid
force-pushed
the
fix/disable-mcp-sse-astra-main
branch
from
03dd1c4 to
fdb4e55
Compare
HzaRashid
force-pushed
the
fix/disable-mcp-sse-astra-main
branch
2 times, most recently
from
fdb4e55 to
850cdbe
Compare
HzaRashid
force-pushed
the
fix/disable-mcp-sse-astra-main
branch
from
22eba75 to
ec16283
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
2 participants
cherry pick #11004.
disables mcp sse transport endpoints if langflow is running in an astra cloud environment.
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.