-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Introduce a function to create a standard AsyncClient with options #655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,64 @@ | ||||||||||
"""Utilities for creating standardized httpx AsyncClient instances.""" | ||||||||||
|
||||||||||
from __future__ import annotations | ||||||||||
|
||||||||||
from typing import Any | ||||||||||
|
||||||||||
import httpx | ||||||||||
|
||||||||||
__all__ = ["create_mcp_http_client"] | ||||||||||
|
||||||||||
|
||||||||||
def create_mcp_http_client( | ||||||||||
headers: dict[str, Any] | None = None, | ||||||||||
|
headers: dict[str, Any] | None = None, | |
headers: dict[str, str] | None = None, |
ref.: https://github.com/encode/httpx/blob/6c7af967734bafd011164f2a1653abc87905a62b/httpx/_models.py#L139
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
headers: dict[str, Any] | None = None, | |
headers: dict[str, str] | None = None, |
ref.: https://github.com/encode/httpx/blob/6c7af967734bafd011164f2a1653abc87905a62b/httpx/_models.py#L139
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
"""Tests for httpx utility functions.""" | ||
|
||
import httpx | ||
|
||
from mcp.shared.httpx_utils import create_mcp_http_client | ||
|
||
|
||
def test_default_settings(): | ||
"""Test that default settings are applied correctly.""" | ||
client = create_mcp_http_client() | ||
|
||
assert client.follow_redirects is True | ||
assert client.timeout.connect == 30.0 | ||
|
||
|
||
def test_custom_parameters(): | ||
"""Test custom headers and timeout are set correctly.""" | ||
headers = {"Authorization": "Bearer token"} | ||
timeout = httpx.Timeout(60.0) | ||
|
||
client = create_mcp_http_client(headers, timeout) | ||
|
||
assert client.headers["Authorization"] == "Bearer token" | ||
assert client.timeout.connect == 60.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this repository follow private/public APIs properly, but maybe it should start...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to use
create_mcp_http_client
across different client transport implementations, so making it private would cause Pyright warnings about accessing private members from other modules.If we want to indicate that the utils module contains internal implementation details, we could consider renaming the file to something like
_internal_httpx_utils.py
. This would signal that the module itself is internal while keeping the functions public for cross-module usage.However, given that this function is in shared/ and genuinely needs to be shared across modules, keeping it public seems more appropriate. Happy to change it if you prefer the internal module approach though or I'm missing something very obvious.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant making the module private_httpx_utils.py
, and keepingcreate_mcp_http_client
.Exactly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not strong, since the rest here doesn't follow it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, thank you! Applied the change. Agree, we need to start somewhere