Is dynamic IHttpClientFactory registration a bug or a feature?

daiplusplus commented 5 years ago

Is your feature request related to a problem? Please describe.

My application needs runtime-defined named HttpClient configuration, and I'm using IHttpClientFactory. My application also needs to configure different DelegatingHandler chains unique to each named configuration (for injecting configuration-specific Bearer tokens and access_token refresh logic).

My application is a headless Windows service that stores 1...N-many named HttpClient configurations on-disk and it uses an injected-service to read the stored configurations, so the IServiceProvider isn't available to read the configuration while it's still calling IServiceCollection.AddHttpClient<MyClient>( ... ) (and the idea of maintaining multiple root IServiceProvider instances isn't appealing either).

Ostensibly, the IHttpClientFactory requires all named HttpClients to be set-up during service-configuration - which would make it unsuitable for my application - however I noticed that DefaultHttpClientFactory's pool of HttpMessageHandler instances can be added-to even after configuration has completed - this does not seem to be documented either way - so I'm not sure if it's a bug (i.e. it should be immutable) or a feature (for runtime-configured HttpClient and HttpMessageHandlerBuilder instances).

Describe the solution you'd like

A statement from the PM responsible declaring the immutability of IHttpClient's configuration.
If it is immutable, then fix the bug.
If it's mutable, then make it easier to add dynamic configuration of HttpClient without jumping through hoops with IConfigureNamedOptions.

Describe alternatives you've considered

Right now my application depends on the current behaviour for dynamic HttpClient configuration, like so:

    internal class DynamicHttpClientFactoryConfiguration : IConfigureNamedOptions<HttpClientFactoryOptions>
    {
        public void Configure( String httpClientName, HttpClientFactoryOptions options )
        {
            IAccessTokenRenewerFactoryStore store = this.sp.GetRequiredService<IAccessTokenRenewerFactoryStore>();

            if( store.TryGetConfiguration( httpClientName: httpClientName, out MyHttpClientConfiguration cfg, out AccessTokenRenewer atr ) )
            {
                options.HttpClientActions.Add( httpClient =>
                {
                    httpClient.BaseAddress = cfg.Authority;
                } );

                options.HttpMessageHandlerBuilderActions.Add( httpMessageHandlerBuilder =>
                {
                    AccessTokenRenewDelegatingHandler delegatingHandler = new AccessTokenRenewDelegatingHandler( atr );
                    httpMessageHandlerBuilder.AdditionalHandlers.Add( delegatingHandler );
                } );
            }
        }
    }

And it "just works" whenever code anywhere in my project calls IHttpClientFactory.CreateClient( configurationName ) (provided that configurationName exists in my IAccessTokenRenewerFactoryStore).

andresrsanchez commented 5 years ago

Holy sh!t, u a f!cking genious 🔥🔥🔥 Please, choose option three!!!

oising commented 4 years ago

Hey @Jehoel -- how exactly do I use DynamicHttpClientFactoryConfiguration ? Where and how do I register it (if needed?)

EDIT: ah, I think I get it: services.ConfigureOptions<DynamicHttpClientFactoryConfiguration>() is needed.

reisenberger commented 4 years ago

@oising As it's an IConfigureNamedOptions<HttpClientFactoryOptions>, I was assuming one had to do:

    services.AddSingleton<IConfigureOptions<HttpClientFactoryOptions>, DynamicHttpClientFactoryConfiguration>();

(but interested if the shorter version works).

It should also be possible to achieve a similar effect with an IHttpMessageHandlerBuilderFilter.

rynowak commented 4 years ago

A statement from the PM responsible declaring the immutability of IHttpClient's configuration. If it is immutable, then fix the bug.

HttpClient factory's configuration is immutable. This is not a bug, it's a feature request.

What's behind your need for N HttpClient configurations? Usually when someone is asking for this kind of feature they asking because they need flexibility related to the use of client certs, proxies, other network settings, etc. Which is it in your case?

oising commented 4 years ago

@rynowak In my case, I've got an event hub and a proxy/relay function that uses information in the message to dynamically determine the REST endpoint to forward the message body. This is represented by a named/typed HttpClient, of which several are cached in a singleton class injected into the function ctor. The problem is that these named clients have to be configured in startup with retry policies, lifetimes etc, when I actually want to configure them dynamically at runtime.

rynowak commented 4 years ago

when I actually want to configure them dynamically at runtime.

When you say you want to configure them dynamically... based on what information? Suppose client factory adds a feature to run code dynamically when a client is requested - what data do you need as input?

daiplusplus commented 4 years ago

@rynowak

HttpClient factory's configuration is immutable. This is not a bug, it's a feature request.

That's not what my post is about. Dynamic configuration already exists, it's just difficult to use.

If the configuration is immutable (as you posit it is), then why is it that I can currently have dynamic configuration using only stock .NET Standard types? (i.e. without using any tricks like reflection, so using only IConfigureOptions<T> and IConfigureNamedOptions).

What's behind your need for N HttpClient configurations?

In my case, I have a single headless Windows Service process that supports multi-tenancy (in the client-side, not just the server-side) so it supports having N-many configuration files which are loaded after ConfigureServices has completed - so each configuration file has its own web-service base URI, client certificate and/or access-token (or OIDC client credentials) and when the process is initialized it means each configuration instance in-memory has its own private HttpClient instance that has the BaseAddress, DefaultRequuestHeaders, and Cookies (via HttpClientHandler) properties set. I couldn't find a good way to get this to work using the stock IHttpClientFactory.

Sidenotes:

I did consider storing the multi-tenant configuration dictionary inside app.config (or a separate file) so it would be exposed via IConfiguration, however a project requirement is that the configuration file be editable and reloadable without restarting the entire process.
- While I know it's possible to use IOptionsSnapshot<T> and other techniques, however conforming to that API would require making substantial (and backwards-incompatible) changes to my existing code.
I also have prior existing library code that uses injected HttpClient instances that I wanted to reuse.
- And this old library code I mentioned must run on embedded systems running rather obscure builds of Windows that aren't supported by .NET Standard 2.0 (downgrading to .NET Standard 1.1-compatible versions of Microsoft.Extensions.* is not feasible).
I also wanted to keep "system configuration" separate from "tenant configuration". The serviceExe.config.json file is kept in C:\Program Files\MyService\ServiceExe.config.json and can only be edited by administrators, whereas the other configuration files are kept in C:\ProgramData\MyService\UserEditableConfig.json and my service's installation code sets the NTFS ACLs on the C:\ProgramData\MyService directory to allow any local user to edit it (this is acceptable given our threat-model).

Here's a feel for what my application's UserEditableConfig.json configuration file looks like:

"serviceClientConfigurations": [
    "customerFoobar": {
        "oidcClientId": "foobar123",
        "oidcClientPassword": "base64value-encrypted-with-DPAPI-machine-key",
        "someOtherSetting": "foobar",
        "pollInterval": 180,
        "webSockets": true
    },
    "customerBaz": {
        "oidcClientId": "baz456",
        "oidcClientPassword": "another-base64value-encrypted-with-DPAPI-machine-key",
        "someOtherSetting": "foobar",
        "pollInterval": 360,
        "webSockets": false
    },
    "customerQux": {
        "oidcClientId": null,
        "oidcClientPassword": null,
        "someOtherSetting": "foobar",
        "pollInterval": 360,
        "webSockets": false
    }
]

oising commented 4 years ago

@rynowak My scenario is roughly the same as @Jehoel above. So, that's a plus one.

ghost commented 4 years ago

As part of the migration of components from dotnet/extensions to dotnet/runtime (https://github.com/aspnet/Announcements/issues/411) we will be bulk closing some of the older issues. If you are still interested in having this issue addressed, just comment and the issue will be automatically reactivated (even if you aren't the author). When you do that, I'll page the team to come take a look. If you've moved on or workaround the issue and no longer need this change, just ignore this and the issue will be closed in 7 days.

If you know that the issue affects a package that has moved to a different repo, please consider re-opening the issue in that repo. If you're unsure, that's OK, someone from the team can help!

daiplusplus commented 4 years ago

@msftbot Please keep this issue open.

Personal rant: I strongly disagree with the practice of closing bugs simply because they're stale or seemingly abandoned. Of course bugs/issues should be tagged as stale over time (which allows them to be filtered ) but in my book, bugs should only be Closed after they're Definitely Resolved - and closing bugs as stale after only 7 days after moving them is a bit mean. There are plenty of issues I've created on GitHub and elsewhere that I can't make progress on because they might be in a completely different project or team that I'm currently working-in and I can't always afford to make the mental context-switch just for a minor update to an issue at very short-notice.

What would be great, especially for API issues like these, if if issues were required to have an attached test-case that demonstrates the issue (if possible). Though requiring people to fork the test project is overkill, perhaps if there was something like an online C# equivalent of LinqPad that could be used to quickly submit quick-and-dirty or proof-of-concept test-cases?

ghost commented 4 years ago

Paging @dotnet/extensions-migration ! This issue has been revived from staleness. Please take a look and route to the appropriate repository.

Vandersteen commented 3 years ago

Are there any plans to ever support this ?

daiplusplus commented 3 years ago

@Vandersteen It is "supported" insofar as it works and Microsoft is duty-bound to not introduce any breaking changes at this point.

However it is "not supported" insofar as the behaviour I've observed in DefaultHttpClientFactory is still not documented.

Vandersteen commented 3 years ago

when I actually want to configure them dynamically at runtime.

When you say you want to configure them dynamically... based on what information? Suppose client factory adds a feature to run code dynamically when a client is requested - what data do you need as input?

In my case:

Configuration is stored in a database (url, auth information, retry policies, certificates, ...)
So I should be able to 'build' a client at runtime and not at 'startup' time

nickjmcclure commented 2 years ago

Has there been any more discussion on this?

Here is the specific example I'm dealing with:

We have an application that pushes users out to our various SAP systems running various SAP products, but running the same base SAP. Our application calls REST web services hosted on the SAP servers to manage users and sync data. When the SAP team sets up a new server, they go to my app to register the server.

In my current .net 4.5 app, I'm just using a singleton with a concurrent dictionary to keep instances of HttpClient, one for each remote system. I use TryGetValue() and GetOrAdd() to keep up with the list. Prior to using this method, connection exhaustion was a major issue.

Is this even needed in .net 6? If I just use the standard HttpClient in my process, will it automatically handle connection pooling based on the client config options like BaseAddress, or do I need to continue to use my singleton to prevent connection exhaustion?

dazinator commented 2 years ago

So I also came to the same sort of solution as posted here, independently. I too, want to be able to re-configure http clients at runtime, for example, changing the base address, or handlers.

I realised the best way to do this is to "cache bust" the IHttpClientFactory by requesting a http client, with a new name, so it builds an entirely new http client configuration. There is nothing in it's design that prohibits this - it's perfectly valid to request a http client with a new name not "registered" ahead of time. The interaction with the Options system is the important missing piece in terms of how such a http client will subsequently be built / configured in terms of handlers etc.

So for example in your application, you would ask for a http client named foo-v1 and then later if there was change to the settings for that http client at runtime, you would need to start asking for foo-v2. The new name ensures the factory doesn't return the previous http client configuration and actually builds a new one. When foo-v1 or foo-v2 or foo-vX is requested, you need to ensure you return a HttpClientFactoryOptions instance configured based on the latest settings for the foo http client. The suffix or prefix mechanism you use isn't exactly important just important to use a mechanism that appends some cache busting value that can be changed at runtime.

The issue with this is that the Options system wants you to supply the names or named options at registration time in order to configure each name. However with a system like this, you don't know all the names at registration time, at runtime the name to be used is incremented and will be therefore, dynamic in nature. So the issue for me became - how do I hook into the Options system such that I can ensure I can configure named Options instances, where the name is changing at runtime, and not specified in advance at registration time.

That's where https://github.com/dazinator/Dazinator.Extensions.Options was born - it supplies this mechansim.

You can then achieve the above like this:

                services.AddHttpClient();

               // Add package: Dazinator.Extensions.Options
                services.Configure<HttpClientFactoryOptions>((sp, name, options) =>
                {
                    // name = e.g "foo-v1", map this to your http client name e.g "foo" and apply the latest configuration.                   
                    options.HttpMessageHandlerBuilderActions.Add(a =>
                    {                       
                        a.PrimaryHandler = new NotImlementedExceptionHttpMessageHandler();
                        // a.BaseAddress = new Uri($"http://{name}.localhost/");
                    });
                });

Note: I did it this way because the ability to register a delegate that can Configure a named options instance, where the name is not known at registration time, seems to be useful in its own right. In the case of this issue with IHttpClientFactory - it proves it's use.

dazinator commented 2 years ago

I've packaged my solution to this: Import nuget package:

    <PackageReference Include="Dazinator.Extensions.Options" Version="0.1.0-alpha.8" />

Add using Microsoft.Extensions.Options; then:

                services.AddHttpClient();              
                services.Configure<HttpClientFactoryOptions>((sp, name, options) =>
                {
                    // configure this named http client however you want:
                    // add to it's handlers:-                
                    options.HttpMessageHandlerBuilderActions.Add(a =>
                    {                      
                        a.PrimaryHandler = new NotImlementedExceptionHttpMessageHandler();                     
                    });
                    // set the http client:
                    options.HttpClientActions.Add(a =>
                    {                       
                        a.BaseAddress = new Uri($"http://{name}.localhost/");
                    });
                });

Then later get clients with the normal IHttpClientFactory - if you want a freshly built client use a new name:

 var fooClient = httpClientFactory.CreateClient("foo-v1");
 var barClient = httpClientFactory.CreateClient("foo-v2");

dazinator commented 2 years ago

I've also solved a few other problems, may be worth reading: https://github.com/dazinator/Dazinator.Extensions.Http

sergiojrdotnet commented 1 year ago

I think we might still need a final decision since the multi tenancy scenario is becoming more frequently for HttpClientFactory

davidfowl commented 1 year ago

@dazinator Clever.

The current factory is unsuitable for this model. Also, the idea of dynamically adding/removing configuration at runtime. The next request that'll come is that the creation of the client will need to be async because tenant information is lazily accessed and needs to queried from the database.

dazinator commented 1 year ago

@davidfowl thanks. It was not clear what the alternative / recommended approach was for this and I was reluctant to give up IHttpClientFactory and write my own factory due to the complexities involved. Luckily the approach was possible within the design, but not very obvious, and perhaps not something the team foresaw. Still it works (for now).

The next request that'll come is that the creation of the client will need to be async because tenant information is lazily accessed and needs to queried from the database

You can make this request already irrespective of anything to do with this issue - if any app wants to configure an http client asynchronously from the database..? I think one reason you may not see demand for this though is because it's already clear to many that IConfiguration / Options system is the configuration story for dotnet apps, so typically you'd expect configuration held in a database (or other async store) to be brought in to IConfiguration (and thus IOptions) by a configuration provider? In this "standard" model, reloading / refreshing IConfiguration before building new objects with it (http clients) becomes the path of least resistance, there would be no need to support an async http client creation method just because of this github issue.

davidfowl commented 1 year ago

I really think it comes down to @rynowak's original question to get a sense of the types of things people are doing or wanting to do per tenant with the HttpClient. If you end up needing to mutate any properties related to the HttpMessageHandler per tenant then everything gets harder, and you need to do what @nickjmcclure described here. Can we enumerate some of these to see what patterns people are using currently?

I'm also very worried about the default scope mismatch between the incoming request (int ASP.NET Core applications) and the outgoing request. This causes lots of issues today and it's something I'd like to address as an overall solution in this space.

Can we pivot to enumerating some of the multitenancy scenarios? We're interested in learning about:

How do you identify the tenant for the outgoing requests?
- Is it ever based on the incoming requests?
Where is tenant information stored?
- Is it all known up front? Is it lazy? Does it require asynchronous fetch to get this information?
Which parts of the HttpClient do you need to change? Client Certs, URLs, headers, etc?
- Is it all per request information or is it per connection information?
Do you need to remove tenant configuration?
- Do you need the per tenant information to expire at some point?

I'm sure there are more questions, but we can start with those.

Vandersteen commented 1 year ago

My 2 cents on the issue:

Maybe you do not need to solve every possible use case for developers, but rather give them the tools to do so. As the code now works, you have a 'factory' that does some caching / pre-configuring for you.

What if, a method is added that accepts:

A 'cache' key
- This key is now automatically generated from configuration or provided by you calling CreateClient(string name);
An (async) callback to construct / configure the client (handlers) if it is not cached
- This essentially already exists today but in form of IOptions / Configuration

The consumer would be responsible to 'bust' the cache key when appropriate, and the IHttpClientFactory can keep doing it's thing without polluting it too much trying to solve the whole world's problems.

As far as I can see, this would be a relatively easy addition in the current code base (without introducing breaking changes).

Creating / configuring a client on the fly: Check
Updating a client on the fly: Check (you create a new one and stop using the old one)
'Removing' a client on the fly: Stop using it, the IHttpClientFactory already has a 'time' based way to dispose / close clients to bust DNS cache (which is also configurable)

One thing that might require 'protecting' is overwriting a 'pre' configured client (if you accidentally use the same cache key as a client configured from startup) Throwing an exception that that is not allowed could be an option there

davidfowl commented 1 year ago

Maybe you do not need to solve every possible use case for developers, but rather give them the tools to do so. As the code now works, you have a 'factory' that does some caching / pre-configuring for you.

Giving developers the right tools mean understanding MANY scenarios and making sure what's there works for them, and for future scenarios they haven't thought about yet.

PS: Finding a hack and making things work for your scenario is fine, but it's not how we rubber stamp scenarios. There are likely issues that haven't been found yet, or ones that break down in other scenarios.

So, let's focus on the what instead of the how, or I can open a new issue to solicit this feedback (since the original issue is about this specific hack).

Vandersteen commented 1 year ago

Giving developers the right tools mean understanding MANY scenarios and making sure what's there works for them, and for future scenarios they haven't thought about yet.

I understand that all too well, my 'impatient' brain kicks in after 3 years working around this issue and is thinking 80/20.

PS: Finding a hack and making things work for your scenario is fine, but it's not how we rubber stamp scenarios. There are likely issues that haven't been found yet, or ones that break down in other scenarios.

To be clear, I'm not talking about a hack like the 'above' mentioned (hacking into the IOptions that is). I might have poorly explained what I meant. But rather re-using internal IHttpClientFactory logic to provide a new 'Factory method' (without using IOptions)

In a poorly explained way, IHttpClientFactory does 2 things: Cache clients and create them. Only today you can only 'pre-warm' the cache using IOptions.

So distill that base logic, expose it and let the IOptions pattern build on top of it

Something like: (No IOptions), an async version might also be considered

CreateClient(string name, Action<HttpClient> configureClient, Func<HttpClientHandler> provideCustomHandler);

You could view that method as 'core' logic, where the following existing one uses the same logic with those 2 arguments prefilled from IOptions

CreateClient(string name);

Not sure if that clarified anything

So, let's focus on the what instead of the how, or I can open a new issue to solicit this feedback (since the original issue is about this specific hack).

I won't sidetrack you with more 'how' after this.

Vandersteen commented 1 year ago

I'm also very worried about the default scope mismatch between the incoming request (int ASP.NET Core applications) and the outgoing request. This causes lots of issues today and it's something I'd like to address as an overall solution in this space.

This one almost bit us as well when we tried to implement a handler using Transient / Scoped services. We were able to work around most except for Client Cert Auth & Trusted SSL Certs

How do you identify the tenant for the outgoing requests?

In a message processing solution i'm working on it is based on the content of the message, sender of the message, ... and the specific configuration required for building the client is stored in a database. Depending on the load going through those 'clients', the ability to react to changes 'quickly' without having to resort to (extremely) short lived / used connections could be helpful performance wise.

Is it ever based on the incoming requests?

Could you clarify, do you mean the ASP.net core request ?

In some solutions yes (We built a reverse proxy using IHttpClientFactory)
In some solutions it's only used in worker services

Where is tenant information stored?

Is it all known up front? Is it lazy? Does it require asynchronous fetch to get this information?

In some solutions everything is known up front
In some solutions it is contained in a store (file / http / db / ...)
- Acquiring this information is almost always async
- (Certificates are the biggest pain point in todays implementation)

Which parts of the HttpClient do you need to change? Client Certs, URLs, headers, etc?

Urls
Headers
Timeout
Not only HttpClient but Handlers as well:
- Client Authentication Certs
- Retry / Throttling Logic
- Trusted SSL certs (ServerCertificateCustomValidationCallback)
- Logging Levels

The following remain untouched in our use cases:

DefaultRequestVersion
DefaultVersionPolicy
MaxResponseContentBufferSize

Is it all per request information or is it per connection information?

We haven't had the need to change configuration on a request to request basis, we are (currently) using multiple connections when that is required. However having the ability to 'react' to changes 'quickly' without rely-ing on expiration would be a welcome feature.

To clarify 'quickly': let the current requests flow through that have a reference to the existing configured client but give me an updated client for new 'scopes', or, let me update a client at run time (if that's possible) In places where we expect 'long processing', we currently resort to IHttpClientFactory and request a new client every X iterations / elapsed timespan

Do you need to remove tenant configuration?

Do you need the per tenant information to expire at some point?

Some configurations could be dropped at some point and never re-used again. Being able to clean up (and free resources) would be a good thing. (We have thousands of different configurations / endpoints in use)

Not all of those are used at the same pace, some are used continuously. Some are used sporadically or once a minute/ hour / day / ...

Being able to 'expire' these at different rates could also save us some resources

dazinator commented 1 year ago

Just wanted to point out obvious that it would cause weird problems to switch out the url (or other settings) for a named http client in current use as im sure everyone is aware. So to do it safely we need to be able to keep old configurations alive whilst being able to rollover to a newer configuration. Using a new http client name works ok for this currently. Whether its a hack or not depends on perspective as there is nothing about IHttpClientFactory that is being altered, its a valid use and extension of the options system in play and falls within the design even if its undocumented. It's just a shame to have to jump through such hoops to get a solution that would be nice to have solved out of the box :-)

dazinator commented 1 year ago

@davidfowl

How do you identify the tenant for the outgoing requests?

outgoing request is made within the scope of a per tenant DI container built when tenant is initialised.

Is it ever based on the incoming requests?

Yes sometimes, if the outgoing request is being made inline with an inbound request, the subdomain from inbound request is used to map the handling of the request to a handler within the scope of the correct tenants container.

Where is tenant information stored?

Database, cached in memory

Is it all known up front? Is it lazy? Does it require asynchronous fetch to get this information?

Yes it's known up front. We either incorporate it from the database into IConfiguration via config Reload() or in some cases we have a service for the tenant that caches the info from database which we expire after settings are changed. No async fetch required usually.

Which parts of the HttpClient do you need to change? Client Certs, URLs, headers, etc.

All of above - we completely rebuild http client after any config changes, url and auth headers are main ones, others like enabling diagnostics enable particular additional handlers in pipeline etc.

Is it all per request information or is it per connection information?

Not sure what this means precisely but think it's both - handlers and url being per connection, optional url path segments being per request level config?

Do you need to remove tenant configuration?

Not explicitly, we need to update it. However we want to do so in way that doesn't disrupt active processes using the previous configuration. This currently means we rotate to a new configuration and trust the old one to 'expire' or be removed after period of non use or after next application restart.

Do you need the per tenant information to expire at some point?

From memory yes but it could technically be required at any point in the future given message queue based processing:-

Two messages placed on queue the first was produced at a time where old http config was in effect and so it's important its payload is sent using this old http config. The second was produced after new http config was in effect so its important its payload gets sent using the new http client config. The application may have been offline for days since the messages were queued. In another scenario both messages are not config version sensitive and can be fine for "the latest" config to be used to process both.

nickjmcclure commented 1 year ago

@davidfowl

How do you identify the tenant for the outgoing requests?

Is it ever based on the incoming requests?

In my case, it isn't a multi-tenancy, my situation is more similar to a webhook type scenario, I have an application that is making outbound calls to a set of sites that are registered to receive updates from our central system. The receiver would be identified by the name of the subscriber, it could be one of several SAP instances that we send updates to.

Where is tenant information stored?

Is it all known up front? Is it lazy? Does it require asynchronous fetch to get this information?

It is stored in a DB table, however subscribers can be added/updated as part of normal runtime, so when a new subscriber comes online, the system will begin to push updates to it. In my situation the subscriber is updated via our front end portal. Then the background system that actually does the work reads the targets when there is an event, if a new target is found, it currently creates a new client and adds it to the dictionary. So the work to pull the config information is already happening, it doesn't really matter if the data is being loaded lazily because that shouldn't impact the registration of the new named HttpClient instance. I already have that config using my current process.

Which parts of the HttpClient do you need to change? Client Certs, URLs, headers, etc?

Is it all per request information or is it per connection information?

For my use it is URLs and Headers. It is per subscriber, so the headers and URLs are always the same per subscriber, so I would have one named HttpClient per subscriber, and that client would have the appropriate configuration for that connection.

Do you need to remove tenant configuration?

Do you need the per tenant information to expire at some point?

Yes, if a subscriber goes away, we'd want to have a method that allowed us to remove an unneeded named HttpClient from the factory.

asturl commented 5 months ago

Can't believe this has been essentially untouched for half a decade. I have a scenario similar to @nickjmcclure and it doesn't seem like a particularly strange or rare requirement to want to define/change named clients after startup.

davidfowl commented 5 months ago

Now that we have keyed services it's much easier to build these sorts of systems using the "any key" pattern.

builder.Services.AddKeyedTransient<HttpClient>(KeyedService.AnyKey, (IServiceProvider sp, object key) =>
{
     // Do something here to get the right http client given the input key. 
});

This avoids the up-front registration problem that the client factory has today.

daiplusplus commented 5 months ago

@davidfowl Now that we have keyed services

I just noticed it's Object key - rather than TObject key - aaaaarggghhhh.

...does ServiceKey use DefaultComparer<Object> then - or does it only use strict reference-equality only - or will any override Boolean Equals(Object? obj) method do? - or how do we specify a custom IEqualityComparer for DI service keys? is String.Empty equivalent to null? Etc? Etc? The documentation page is unhelpfully sparse on details.

davidfowl commented 5 months ago

It uses object.Equals by default but for this you just need strings.

spencer741 commented 4 months ago

Bump. Post-Container-Registration needs:

Add/Remove Client Authentication Certs
Add/Remove headers
Modify Request Timeouts

(In case you want to know...Mainly driven by hot-reloading fields from config files)

Keep the train going? IHttpClientFactoryFactory /s

julealgon commented 4 months ago

Now that we have keyed services it's much easier to build these sorts of systems using the "any key" pattern.
builder.Services.AddKeyedTransient<HttpClient>(KeyedService.AnyKey, (IServiceProvider sp, object key) =>
{
     // Do something here to get the right http client given the input key. 
});
This avoids the up-front registration problem that the client factory has today.

I feel dumb now for not knowing this AnyKey even existed! Thanks for sharing that @davidfowl .

I've had scenarios in the past similar to some that were described here, where we would allow API consumers to register HTTP callbacks dynamically with our API to notify them back of some action. Obviously, we'd want for each of those HTTP connections to be managed separately by IHttpClientFactory so lifetime was taken care of properly for each of them.

Can you elaborate how one would use the AnyKey approach while still leveraging IHttpClientFactory behind the scenes? Would that still allow for changes to an existing HttpClient after the initial creation/configuration, or would we need to do some "versioning in the name" like what @dazinator proposed a few posts above?

justinpenguin45 commented 1 month ago

@davidfowl apologies but can you elaborate on how we would use the keyed approach to solve for HttpClients that depend on request scoped data. In my scenario I need to be able to provide HttpClients with a BaseAddress that is dependent on data that is part of the incoming request (auth token has an organization ID) - we use the organization ID to lookup a base address for the HttpClient. Can what you proposed be used to solve this? This needs to be done at runtime and not at service startup

dotnet / runtime