-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Is dynamic IHttpClientFactory registration a bug or a feature? #36378
Comments
Holy sh!t, u a f!cking genious 🔥🔥🔥 |
Hey @Jehoel -- how exactly do I use DynamicHttpClientFactoryConfiguration ? Where and how do I register it (if needed?) EDIT: ah, I think I get it: |
@oising As it's an services.AddSingleton<IConfigureOptions<HttpClientFactoryOptions>, DynamicHttpClientFactoryConfiguration>(); (but interested if the shorter version works). It should also be possible to achieve a similar effect with an |
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? |
@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. |
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? |
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
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 Sidenotes:
Here's a feel for what my application's
|
As part of the migration of components from dotnet/extensions to dotnet/runtime (aspnet/Announcements#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! |
@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? |
Paging @dotnet/extensions-migration ! This issue has been revived from staleness. Please take a look and route to the appropriate repository. |
Are there any plans to ever support this ? |
@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 |
In my case:
|
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 Is this even needed in .net 6? If I just use the standard |
So I also came to the same sort of solution as posted here, independently. I realised the best way to do this is to "cache bust" the So for example in your application, you would ask for a http client named The issue with this is that the 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 |
I've packaged my solution to this: Import nuget package:
Add 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 var fooClient = httpClientFactory.CreateClient("foo-v1");
var barClient = httpClientFactory.CreateClient("foo-v2"); |
I've also solved a few other problems, may be worth reading: https://github.com/dazinator/Dazinator.Extensions.Http |
I think we might still need a final decision since the multi tenancy scenario is becoming more frequently for |
@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. |
@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).
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. |
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 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:
I'm sure there are more questions, but we can start with those. |
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. What if, a method is added that accepts:
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).
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) |
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). |
I understand that all too well, my 'impatient' brain kicks in after 3 years working around this issue and is thinking 80/20.
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
You could view that method as 'core' logic, where the following existing one uses the same logic with those 2 arguments prefilled from IOptions
Not sure if that clarified anything
I won't sidetrack you with more 'how' after this. |
This one almost bit us as well when we tried to implement a handler using Transient / Scoped services.
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.
Could you clarify, do you mean the ASP.net core request ?
The following remain untouched in our use cases:
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)
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. 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 |
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. |
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.
Database, cached in memory
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.
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.
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?
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.
From memory yes but it could technically be required at any point in the future given message queue based processing:-
|
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.
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.
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.
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. |
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. |
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 just noticed it's ...does |
It uses object.Equals by default but for this you just need strings. |
Bump. Post-Container-Registration needs:
(In case you want to know...Mainly driven by hot-reloading fields from config files) Keep the train going? IHttpClientFactoryFactory /s |
I feel dumb now for not knowing this 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 Can you elaborate how one would use the |
@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 |
Is your feature request related to a problem? Please describe.
My application needs runtime-defined named
HttpClient
configuration, and I'm usingIHttpClientFactory
. My application also needs to configure differentDelegatingHandler
chains unique to each named configuration (for injecting configuration-specific Bearer tokens andaccess_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 theIServiceProvider
isn't available to read the configuration while it's still callingIServiceCollection.AddHttpClient<MyClient>( ... )
(and the idea of maintaining multiple rootIServiceProvider
instances isn't appealing either).Ostensibly, the
IHttpClientFactory
requires all namedHttpClient
s to be set-up during service-configuration - which would make it unsuitable for my application - however I noticed thatDefaultHttpClientFactory
's pool ofHttpMessageHandler
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-configuredHttpClient
andHttpMessageHandlerBuilder
instances).Describe the solution you'd like
IHttpClient
's configuration.HttpClient
without jumping through hoops withIConfigureNamedOptions
.Describe alternatives you've considered
Right now my application depends on the current behaviour for dynamic
HttpClient
configuration, like so:And it "just works" whenever code anywhere in my project calls
IHttpClientFactory.CreateClient( configurationName )
(provided thatconfigurationName
exists in myIAccessTokenRenewerFactoryStore
).The text was updated successfully, but these errors were encountered: