Skip to content

feat!: Deprecate SSEServerInfo base_url in favour of url#1662

Merged
vblagoje merged 11 commits into
mainfrom
deprecate_base_url_mcp_tool
May 6, 2025
Merged

feat!: Deprecate SSEServerInfo base_url in favour of url#1662
vblagoje merged 11 commits into
mainfrom
deprecate_base_url_mcp_tool

Conversation

@vblagoje
Copy link
Copy Markdown
Member

@vblagoje vblagoje commented Apr 21, 2025

Why

This PR deprecates the base_url parameter in favor of a single url argument for configuring the SSE client. It adds deprecation warnings and updates relevant classes to reflect this change.

This change aligns with common usage patterns—users are typically more familiar with providing the full URL including the /sse endpoint, and omitting it (via base_url) can be unintuitive or error-prone. Simplifying the interface improves clarity and consistency.

Details

  • Adds a deprecation warning if base_url is used.
  • Ensures that only one of url or base_url can be passed.
  • Updates both SSEClient and SSEServerInfo to enforce and reflect this change.

Deprecation Notice

base_url is now deprecated and will be removed in a future release. Please switch to using the full url (including the /sse path) when initializing the client.

Checklist

  • Backward compatibility maintained (with warning)
  • Clear error handling for conflicting or missing parameters
  • Docstrings and inline comments updated

@github-actions github-actions Bot added integration:mcp type:documentation Improvements or additions to documentation labels Apr 21, 2025
@vblagoje vblagoje marked this pull request as ready for review April 21, 2025 12:21
@vblagoje vblagoje requested a review from a team as a code owner April 21, 2025 12:21
@vblagoje vblagoje requested review from anakin87 and mpangrazzi and removed request for a team April 21, 2025 12:21
@vblagoje
Copy link
Copy Markdown
Member Author

vblagoje commented Apr 21, 2025

@anakin87 @mpangrazzi is already familiar with this discussion - it is a small change that will improve devX as users are mostly used to providing MCP server URL with /sse suffix rather than just base_url and intuitively seem more future proof.

@vblagoje
Copy link
Copy Markdown
Member Author

cc @bilgeyucel this is the devX upgrade we talked about

@vblagoje
Copy link
Copy Markdown
Member Author

@anakin87 @mpangrazzi can you please take another look now? There are some minor internal simplifications that are not public and make everything much simpler

@anakin87
Copy link
Copy Markdown
Member

anakin87 commented May 2, 2025

I am ignoring this PR because I thought that Michele had more context. In case it is urgent, please let me know.

@vblagoje
Copy link
Copy Markdown
Member Author

vblagoje commented May 2, 2025

@anakin87 np, @mpangrazzi has all the context and @oryx1729 can also chime in with his UI/UX experience around this matter

Copy link
Copy Markdown
Contributor

@mpangrazzi mpangrazzi left a comment

Choose a reason for hiding this comment

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

LGTM! - I've left only a minor comment

Comment thread integrations/mcp/src/haystack_integrations/tools/mcp/mcp_tool.py Outdated
@vblagoje vblagoje merged commit e130d30 into main May 6, 2025
3 checks passed
@vblagoje vblagoje deleted the deprecate_base_url_mcp_tool branch May 6, 2025 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration:mcp type:documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants