Conversation
| public {{packageName}}Configuration(string basePath, | ||
| HttpClientHandler httpClientHandler = null, | ||
| public VaultConfiguration(string basePath, | ||
| Func<HttpClientHandler, HttpClient> httpClientProvider = null, |
There was a problem hiding this comment.
what is Func here? shouldn't this be HttpClient?
(sorry if dumb question, looks weird from a non-c# perspective)
I saw IHttpClientFactory above, shouldn't it take that as the param type?
There was a problem hiding this comment.
So, this is kind of weird. But because the VaultClient needs to configure the HttpClientHandler (for TLS among other things), which the HttpClient is dependent on we have to allow users to pass a delegate (i.e. Func) to new up their own HttpClient with our pre-configured client handler.
There was a problem hiding this comment.
I was debating whether to allow just to pass their own HttpClient object, like you're suggesting, but if they wanted to utilize the TLS, then they'd have to preconfigure it, which may be annoying.
It could be that we offer both options.
There was a problem hiding this comment.
In the Go version, we allow you to provide HTTPClient directly but then override the TLS options (if configured). Not sure if this is something doable here.
There was a problem hiding this comment.
Does this make sense in the context of IHttpClientFactory, as the factory manages the HttpClientHandler instances?
| We also allow you to bring your own `HttpClient` through a delegate. This can work | ||
| with any current factory pattern that you're using to create an `HttpClient` | ||
| object, which is one of the recommended patterns for managing your `HttpClient` | ||
| lifecycle. More on the `IHttpClientFactory` can be found [here][[http-client-factory]] |
Description
This allows users to bring their own
HttpClientinstead ofHttpClientHandler. This gives them more control over managing the lifecycle of theHttpClientif they choose, which is notoriously difficult in .Net. It also aligns with the recommended factory pattern of creating HttpClients recommended hereIf you set the
timeoutwhen configuringVaultClientthat will take precedent over any timeout configured in your providedHttpClientResolves # VAULT-9887
How has this been tested?
Tested locally with a generic
HttpClientand preconfigured with some random settings.