search

ngocnicholas / airtable.net

Airtable .NET API Client
MIT License
138 stars 34 forks source link

[feat] HttpClient IOC #83

Closed sommmen closed 3 weeks ago

sommmen commented 6 months ago

Hiya,

It's genreally considered 'bad' to create new HttpClients everytime. Could we get an overload ctor where we can specify a HttpClient or IHttpClientfactory instead of AirtableBase newing a HttpClient everytime?

ngocnicholas commented 5 months ago

This will be implemented in the next update. Thanks for your input.

ngocnicholas commented 1 month ago

Hi Sommen, I'm looking into this issue some more. I have second thought about making this change. Below is the remarks of HttpClient provided by Microsoft. HttpClient is intended to be instantiated once and re-used throughout the life of an application. Instantiating an HttpClient class for every request will exhaust the number of sockets available under heavy loads. This will result in SocketException errors. Airtable.net instantiates one AirtableBase which instantiates HttpClient only once. All calls to APIs of Airtable.net via AirtableBase use this same HttpClient throughout the life of AirtableBase. I won't make any changes while waiting for your reply with a sufficiently good reason for this change. Thanks for your input.

sommmen commented 1 month ago

hiya @ngocnicholas - The provided example is exactly why it is considered bad in some cases to create a new client everytime. Have you seen: https://learn.microsoft.com/en-us/dotnet/fundamentals/networking/http/httpclient-guidelines#recommended-use ?

The AirtTableBase seems to be appropriate for transient use, i.e. you create a base, work with it and then dispose it (as per example). In this case the httpclient is not used as a singleton, and thus i can create a lot of AirtTableBase and it will cause port exhaustion.

Regardless of this it does not matter, i'm talking about an architectural change where you leave this control up to the user; You leave the base ctor as-is, and provide a second one that takes an IHttpClientFactory and/or HttpClient and the user has all the freedom to decide.

For example, lets say i want to add resillience to the airtable calls? https://learn.microsoft.com/en-us/dotnet/core/resilience/http-resilience?tabs=dotnet-cli#add-standard-resilience-handler Now i can't do that because the HttpClient is in no way exposed.

This is especially useful for modern apps where ms DI is used.

Let me know your thoughts.

ngocnicholas commented 3 weeks ago

An overload ctor is added to support this feature.