Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Updated RestRequest to use http client factory rather than http client directly #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nicklundin08
Copy link

Currently the RestClients generated hold onto the http client for the lifecycle of the client. This change would allow the user to pass in something else that manages the lifecycle of the http client

e.g. IHttpClientFactory https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-2.2.

#2

@nicklundin08
Copy link
Author

After sleeping on this I realize the problems my PR is trying to solve isn't quite clear. I've expanded on this a bit here

My thought process behind using Func rather than the IHttpClientFactory directly was that the library would not have to add the dependency on this package (which requires .net standard 2.0). However, I think one of the downfalls of this approach is that the client can easily shoot themselves in the foot with Func

Perhaps a better solution would be to have a separate package (RestLess.Extensions.Http) or something like that that has an entry point (maybe RestClient becomes a partial class that also exists in this package so that the apis look similar from the clients perspective) that supports the interface directly (but under the hood the rest request and all the other classes still use Func that way they can support using either a single instance of HttpClient or using IHttpClientFactory to create an HttpClient for each request).

Ill think about that for a little bit and I may end up editing the PR, but in the meantime if you have any feedback I'd love to hear it!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant