Skip to content
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

Transport: Fix unify HttpClient usage across Gateway classes #1548

Merged
merged 19 commits into from Jul 20, 2020

Conversation

ealsur
Copy link
Member

@ealsur ealsur commented May 20, 2020

Description

#1429 added support for HttpClientFactory inside the gateway connection mode, but the DocumentClient was maintaining two other HttpClient instances on GatewayAccountReader and GatewayAddressCache.

If the user was trying to use a custom HttpClientFactory to use a single HttpClient instance, it was not reaching these classes.

The effect also was that we were maintaining 3 different HttpClient instances normally.

This PR unifies this into a single HttpClient, that can either be created or obtained from the HttpClientFactory.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

@ealsur
Copy link
Member Author

ealsur commented May 20, 2020

Turns out it's a bit more tricky, we are using the DefaultHeaders on the HttpClient in many places, so if we wire the same HttpClient across all these places, then the DefaultHeaders might have duplicate values. Still investigating.

@ealsur ealsur changed the title Transport: Fix HttpClientFactory on GatewayAccountReader Transport: Fix unify HttpClient usage across Gateway classes May 21, 2020
@kirankumarkolli kirankumarkolli added this to Reviewer approved in Cosmos DB SDK team via automation Jun 11, 2020
Cosmos DB SDK team automation moved this from Reviewer approved to Review in progress Jul 3, 2020
Cosmos DB SDK team automation moved this from Review in progress to Reviewer approved Jul 20, 2020
@ealsur ealsur merged commit daf7a8c into master Jul 20, 2020
Cosmos DB SDK team automation moved this from Reviewer approved to Done Jul 20, 2020
@ealsur ealsur deleted the users/ealsur/accountreader branch July 20, 2020 20:14
fnarenji pushed a commit that referenced this pull request Oct 13, 2020
If the user was trying to use a custom HttpClientFactory to use a single HttpClient instance, it was not reaching these classes.

The effect also was that we were maintaining 3 different HttpClient instances normally.

This PR unifies this into a single HttpClient, that can either be created or obtained from the HttpClientFactory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants