-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Replace httpx_client_factory with direct httpx_client parameter for better flexibility #1280
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
Conversation
We should be passing the client, instead of allowing more and more options. |
…ead of a separate verify param.
…eature/ssl-verification
Respecting the original author's intention to avoid exposing AsyncClient directly to users, the FactoryClient was modified to accept kwargs. |
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.
IIUC I don't think the intention here was to pass through arbitrary arguments through to the underlying client, but refactoring to remove the factory in the first place and explicitly use the client.
See #1177 for context. I'm going to mark this as pending that change.
I’d greatly appreciate any active and constructive reviews. |
Hi @ruhz3 thank you for updating this PR - I don't think we'll go with this approach as we're actually intending to remove all the arguments other than the We're currently working on this here: #1177 I'm closing this for now, feel free to chime in on the linked PR though if it's missing something you need! |
Added support for the
verify
option to thecreate_mcp_http_client
function and related client factories, allowing users to control SSL certificate verification when creating an httpx AsyncClient.Motivation and Context
Some users need to disable SSL verification (e.g., for testing or internal endpoints), or use a custom CA bundle. This change makes it possible to pass
verify=True
,verify=False
, or a customssl.SSLContext
when creating MCP HTTP clients.How Has This Been Tested?
Tested by creating AsyncClient instances with different
verify
values (True
,False
, and custom context) and confirming correct behavior for secure and insecure endpoints. Existing unit tests for HTTP client creation were also run.Breaking Changes
No breaking changes. The new
verify
parameter is optional and defaults to the previous behavior.Types of changes
Checklist
Additional context
This change improves flexibility for users working in environments with custom or self-signed certificates, or those who need to disable SSL verification for
resolves #870
This PR does not fully resolve the linked issue, but it applies the necessary modifications to the current repository. Further functional modifications following this PR are needed in the fastmcp repository.