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: Add HttpClientFactory support #1441

Merged
merged 11 commits into from
May 1, 2020
Merged

Conversation

ealsur
Copy link
Member

@ealsur ealsur commented Apr 28, 2020

This PR adds support for the user to provide a factory for HttpClient instances.

Scenarios

When building ASP.NET Core applications on NET Core 2.1/3, users can take advantage of IHttpClientFactory to reuse HttpClient instances across the application. This also allows HttpClient instances to be reused across CosmosClient instances.
Benefits of using IHttpClientFactory on ASP.NET Core applications: https://docs.microsoft.com/en-us/dotnet/architecture/microservices/implement-resilient-applications/use-httpclientfactory-to-implement-resilient-http-requests#benefits-of-using-ihttpclientfactory

When building Blazor applications on WebAssembly, the applications use a preconfigured HttpClient instance that needs to be injected (reference https://docs.microsoft.com/en-us/aspnet/core/blazor/call-web-api?view=aspnetcore-3.1)

API

This PR adds HttpClientFactory to CosmosClientOptions:

public Func<HttpClient> HttpClientFactory { get; set; }

And WithHttpClientFactory to CosmosClientBuilder:

public CosmosClientBuilder WithHttpClientFactory(Func<HttpClient> httpClientFactory)

To provide a delegate that matches IHttpClientFactory.CreateClient.

The new API is not compatible with CosmosClientOptions.WebProxy and if the user tries to set both, there will be an ArgumentException.

Why not use IHttpClientFactory on the API?

To avoid a reference to Microsoft.Extensions.Http on the Cosmos DB SDK package unnecessarily.

Usage

In a Blazor WebAssembly application:

public static async Task Main(string[] args)
        {
            var builder = WebAssemblyHostBuilder.CreateDefault(args);
            builder.RootComponents.Add<App>("app");

            builder.Services.AddTransient(sp => new HttpClient { BaseAddress = new Uri(builder.HostEnvironment.BaseAddress) });

            builder.Services.AddHttpClient();

            builder.Services.AddSingleton<CosmosClient>(sp =>
            {
                var httpClientFactory = sp.GetService<IHttpClientFactory>();
                CosmosClientOptions cosmosClientOptions = new CosmosClientOptions();
                cosmosClientOptions.HttpClientFactory = httpClientFactory.CreateClient;
                cosmosClientOptions.ConnectionMode = ConnectionMode.Gateway;
                return new CosmosClient("<connection-string>", cosmosClientOptions);
            });

            await builder.Build().RunAsync();
        }

In an ASP.NET Core application:

public void ConfigureServices(IServiceCollection services)
{
    // other service configuration
    // adds in HttpClientFactory
    services.AddHttpClient();
    services.AddSingleton<CosmosClient>(sp =>
            {
                var httpClientFactory = sp.GetService<IHttpClientFactory>();
                CosmosClientOptions cosmosClientOptions = new CosmosClientOptions();
                cosmosClientOptions.HttpClientFactory = httpClientFactory.CreateClient;
                cosmosClientOptions.ConnectionMode = ConnectionMode.Gateway;
                return new CosmosClient("<connection-string>", cosmosClientOptions);
            });
}

Related to #1429. One more PR is required to fully support Blazor WebAssembly, but this is a good first step.

Tracking usage

The PR also adds a new Feature on the user agent to track and identify usage of the feature.

Type of change

  • New feature (non-breaking change which adds functionality)

j82w
j82w previously approved these changes Apr 28, 2020
@kirankumarkolli
Copy link
Member

        httpClient.Timeout = (requestTimeout > this.requestTimeout) ? requestTimeout : this.requestTimeout;

Overriding these on CX given client seems like overstepping each other.
Any other possibilities?


Refers to: Microsoft.Azure.Cosmos/src/GatewayStoreModel.cs:111 in a4ae553. [](commit_id = a4ae553, deletion_comment = False)

Copy link
Member

@kirankumarkolli kirankumarkolli left a comment

Choose a reason for hiding this comment

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

Overriding CX given HttpClient seems like overstepping each other.
Any other possibilities?

@ealsur
Copy link
Member Author

ealsur commented Apr 29, 2020

@kirankumarkolli If the user is setting the RequestTimeout through ClientOptions and also providing an HttpClient from the factory, I don't see the issue of configuring the provided instance.

We have 2 choices, either assume the user would configure the timeout, or not.

If we target the most common scenarios for users to use this API, which are described in the Description, then the user would not be configuring the timeout there, but they could be setting the timeout on the ClientOptions and I would expect the Options to be honored.

If the user is indeed configuring a custom timeout on the HttpClientFactory, and they expect that to be honored, we just tell them that they can configure that through the ClientOptions, as they can do right now.

@ealsur
Copy link
Member Author

ealsur commented Apr 29, 2020

@kirankumarkolli I added a new Feature flag on the UserAgent, so we can identify when a user is using their own HttpClientFactory

@ealsur ealsur merged commit 5024c15 into master May 1, 2020
@ealsur ealsur deleted the users/ealsur/httpfactory branch May 1, 2020 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants