Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix reviews
Signed-off-by: aaronzuo <anarionzuo@outlook.com>
  • Loading branch information
Anarion-zuo authored and ntkathole committed Apr 5, 2026
commit face1ed3fc5c25ef88a7d573632e58863610a36c
3 changes: 2 additions & 1 deletion .github/workflows/pr_integration_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ jobs:
SERVER_PID=$!
echo $SERVER_PID > /tmp/feast_server_pid
for i in $(seq 1 60); do
kill -0 "$SERVER_PID" || { echo "server died"; exit 1; }
if curl -fsS http://127.0.0.1:6566/health >/dev/null; then
Comment thread
franciscojavierarceo marked this conversation as resolved.
break
fi
Expand All @@ -143,7 +144,7 @@ jobs:
-H "Content-Type: application/json" \
-H "mcp-protocol-version: 2025-03-26" \
--data '{}' \
http://127.0.0.1:6566/mcp || true
http://127.0.0.1:6566/mcp

SESSION_ID=$(grep -i "^mcp-session-id:" /tmp/mcp_headers | head -1 | awk '{print $2}' | tr -d '\r')
if [ -z "${SESSION_ID}" ]; then
Expand Down
2 changes: 1 addition & 1 deletion sdk/python/feast/feature_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ def _add_mcp_support_if_enabled(app, store: "feast.FeatureStore"):
)

mcp_transport_not_supported_error = McpTransportNotSupportedError
except Exception as e:
except ImportError as e:
logger.error(f"Error checking/adding MCP support: {e}")
return

Expand Down
6 changes: 4 additions & 2 deletions sdk/python/feast/infra/mcp_servers/mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def add_mcp_support_to_app(app, store: FeatureStore, config) -> Optional["FastAp
)

transport: Literal["sse", "http"] = (
getattr(config, "mcp_transport", "sse") or "sse"
getattr(config, "mcp_transport", "sse")
)
if transport == "http":
mount_http = getattr(mcp, "mount_http", None)
Expand All @@ -60,8 +60,10 @@ def add_mcp_support_to_app(app, store: FeatureStore, config) -> Optional["FastAp
if mount_sse is not None:
mount_sse()
else:
logging.warning("transport sse not supported, fallback to the deprecated mount().")
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Outdated
mcp.mount()
Comment thread
franciscojavierarceo marked this conversation as resolved.
else:
# Code should not reach here
raise McpTransportNotSupportedError(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This branch is unreachable — Literal["sse", "http"] in the config rejects anything else at parse time. Either remove it or add a comment that it's a defensive guard for programmatic callers.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@Anarion-zuo u didn't solve it btw :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Anarion-zuo any update on this one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry. Missed this one. Will look at it ASAP.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@YassinNouh21 Made some changes. Is this aligned with what you intended?

f"Unsupported mcp_transport={transport!r}. Expected 'sse' or 'http'."
)
Expand All @@ -79,5 +81,5 @@ def add_mcp_support_to_app(app, store: FeatureStore, config) -> Optional["FastAp
except McpTransportNotSupportedError:
raise
except Exception as e:
logger.error(f"Failed to initialize MCP integration: {e}")
logger.error(f"Failed to initialize MCP integration: {e}", exc_info=True)
return None