-
Notifications
You must be signed in to change notification settings - Fork 345
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
Add an extension method to add Dapr HTTP Client to service collection #632
Comments
Thanks @karoldeland - this makes what you're proposing more clear. Can you answer one question for me? What features of the client factory do you need for your scenario?
I'm yet convinced that this is the right thing to do on a few axes:
Personally, my approach for these things is avoid over-indexing on whether something is one line of code or multiple. Providing a convenience API for helps when you want users to find a feature without documentation, or when the task is so shocking common that it's going to help 95% of people for 95% of their use cases. I think what would change my mind would be:
To continue this discussion, here's an example of what this might look like today if I put it in the docs without adding any new features. public void ConfigureServices(IServiceCollection services)
{
services.AddHttpClient<CartClient>(c =>
{
c.BaseAddress = new Uri("http://cart");
})
.AddHttpMessageHandler(() => new InvocationHandler());
} Depending on your answers to the questions I asked, here's an even simpler example without the factory: public void ConfigureServices(IServiceCollection services)
{
services.AddSingleton<CartClient>(new CartClient(DaprClient.CreateInvokeHttpClient("cart"));
} |
Hi @rynowak, For me, the need was mainly a concern of adopting the best-practices for ASP.NET about the IHttpClientFactory. It will be useful to have the possibility to add global policies like retries, but it seems that Dapr will solve this early soon. So I think it's not a good idea to add code in the SDK that have high chance to be useless in the short term. IMHO, the second sample you provide is nice. I personaly don't like to have http:// strings in the code. For sure, adding this to documentation would be a great addition. |
Ok, thanks @karoldeland - with that resolved I'm going to close your PR and treat this as a docs issue. Feel free to discuss more if you have other ideas. |
@rynowak Are you looking for any help? I would love to start my open source journey with this as am really interested in Dapr project. |
@rynowak I hope all is well. I realize this thread is over a year old but I have a similar need.. I am trying to upgrade an existing GraphQL framework to run on Dapr. Pre-Dapr ExistingIn the Pre-Dapr configuration there is a GraphQL Gateway service, which uses Named Http Clients, via a HttpClientFactory to each HttpClient per each downstream service as follows... Add Http Endpoints ...
Note : the AddHeaderPropagation() at the end of each HttpClient add, more on that in a second.. When a GraphQL query is issued to the Gateway it will craft up a query plan and invoke the necessary query(s) to each down-stream GraphQL service. It will execute each query through the specific, previously configured HttpClient as follows;
Where the targetSchema in this example is either "OrderServices" or "InventoryServices". Using-DaprWhen converting to Dapr, I will be running the Gateway and all downstream services via Dapr Side Cars and as such do not need the HttpClientFactory, as I will create the HttpClient in the following way, in which the targetSchema is the Dapr AppId....
The solution would then look like this.. The above works great without applied security, however I am further utilizing Azure B2C to secure both the clients as well as the all services (including the Gateway). As such, I would like to propagate the "authorization" header (JWT) that is received at the Gateway down to each downstream service. Pre-Dapr I was using Microsoft.AspNetCore.HeaderPropagation and this works perfectly to propagate the "authorization" (JWT) from the client->gateway->service1/2/etc.. That said, you'll note that the way I was able to make that work was adding the Given that Darp Client is creating the HttpClient, I was expecting it would also propagate the Headers I configured to propagate but sadly no. Is there a way to add the Dapr produced Http Client into the HttpClientFactory? In that way I may be able to take advantage of Header propagation. If not that way, can you please suggest another way? Many thanks in advance, John |
@rynowak Actually I got it to work based upon your example....
Very simple so thank you! |
Describe the proposal
Currently, we can call the CreateInvokeHttpClient from DaprClient to create a preconfigured HttpClient. It would be great to leverage the IHttpClientFactory of ASP.NET Core with the same level of integration as the CreateInvokeHttpClient.
In short, there's how it could be used :
services.AddDaprHttpClient<CartClient, CartClient>("appId");
I've created a PR to see how it could be implemented.
The text was updated successfully, but these errors were encountered: