reuse SSL context and API http client instead of rebuilding per request - #535
Draft
lilyydu wants to merge 1 commit into
Draft
reuse SSL context and API http client instead of rebuilding per request#535lilyydu wants to merge 1 commit into
lilyydu wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a performance fix concerning redundant HTTP client construction. Callers can now share a pre-built SSL context instead of paying ~2.4ms of CA parsing per client, and the API client is built once instead of on every inbound activity - which is what restores TCP connection reuse.
Problem
Client.__init__always builds its ownhttpx.AsyncClient, which constructs a freshssl.SSLContext- parsing the full CA bundle (~150 certs) every time.ssl.create_default_context()- 2.37mshttpx.AsyncClient()- 2.45ms (almost entirely the line above)Two consequences:
ClientOptionshad no hook for supplying a context, andclone()rebuilt from options only.ActivityProcessor._build_contextcloned the http client on every inbound activity, even though the clone was identical each time:token_manager.get_bot_tokenis the same bound method, takes no arguments, and is resolved per request anyway. Every activity therefore got a brand-new connection pool, so no outbound call to Teams ever reused a TCP connection.Changes
ClientOptions.verify: Optional[ssl.SSLContext]- opt-in. Callers may pass a pre-built context;clone()propagates it.ActivityProcessor- build the API http client once in__init__rather than per activity.Impact
Why
verify=and nottransport=Passing a shared
transportalso skips SSL setup, but silently disables httpx's env-based proxy detection and collapses pool isolation.verify=keeps both.verify=ctxtransport=Compatibility
Default behaviour is unchanged -
verifydefaults toNone, which maps to httpx's ownTrue. The field is typedOptional[ssl.SSLContext], deliberately not acceptingbool, soverify=Falsecan't be smuggled through to disable TLS verification.Reviewer caveats
ctx.api.http.use_interceptor(...)used to be discarded with the per-activity clone; it now accumulates across activities. Nothing in the SDK or examples calls it, but it is a real semantic change.