Skip to content
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.

Sign up for GitHub

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

Jump to bottom

MINOR FIX: PENDING New Version of RESTFulSense with HttpFactory #20

Open
jodendaal opened this issue Mar 13, 2023 · 6 comments
Open

MINOR FIX: PENDING New Version of RESTFulSense with HttpFactory #20

jodendaal opened this issue Mar 13, 2023 · 6 comments

Comments

@jodendaal
Copy link

If creating a HttpClient manually, i.e new HttpClient() that is either disposed frequently or is long running,it is recommended to create your own [HttpMessageHandler] and handle lifetimes etc to avoid socket exhaustion and DNS update failures. I would recommend just using the IHttpClientFactory to create new http instances which handles all of this really well.

I have dealt with this issue quite a few times in production environments with long running high through put scenarios. It's not an obvious issue, but eventually rears its ugly head.

Further Reading

https://learn.microsoft.com/en-us/dotnet/architecture/microservices/implement-resilient-applications/use-httpclientfactory-to-implement-resilient-http-requests

https://www.aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/
@hassanhabib
Copy link
Owner

@BrianLParker

@hassanhabib hassanhabib changed the title Check Usage of HttpClient MINOR FIX: Check Usage of HttpClient Mar 15, 2023
@hassanhabib hassanhabib changed the title MINOR FIX: Check Usage of HttpClient CODE RUB: Check Usage of HttpClient Mar 15, 2023
@BrianLParker
Copy link
Collaborator

BrianLParker commented Mar 16, 2023 •
edited
Loading

http://byterot.blogspot.com/2016/07/singleton-httpclient-dns.html

Use the HttpClientFactory.
The HttpClientFactory keeps its own pool of HttpClientHandlers and recycles them periodically.

The library should inject the HttpClientFactory or request it as a required service, and then create a configured client that can be reused for the OpenAI endpoint.

Thankyou @jodendaal for pointing this out.

@hassanhabib
Copy link
Owner

@jodendaal please create an issue on the RESTFulSense Project and change the name for this one to MINOR FIX: PENDING New Version of RESTFulSense with HttpFactory

cc: @cjdutoit

@jodendaal jodendaal changed the title CODE RUB: Check Usage of HttpClient MINOR FIX: PENDING New Version of RESTFulSense with HttpFactory Mar 18, 2023
@rafsanulhasan rafsanulhasan mentioned this issue Mar 19, 2023
Closed
@rafsanulhasan
Copy link

rafsanulhasan commented Mar 19, 2023 •
edited
Loading

@hassanhabib I've just created a PR #77 demonstrating how to use IHttpClientFactory from Microsoft.Extensions.Http. We can use that library to register both our http client and IRESTFulClientFactory in DI. I think this way we can have a cleaner code in the OpenAIBroker.

@hassanhabib
Copy link
Owner

@rafsanulhasan every broker should contain it's own dependencies. Brokers should only receive configurations and initialize it's own external libraries.

Refer to this portion here in The Standard:
https://github.com/hassanhabib/The-Standard/blob/master/1.%20Brokers/1.%20Brokers.md#123-own-their-configurations

We should have bits and pieces of Broker's requirements spread across the app.

@rafsanulhasan rafsanulhasan mentioned this issue Mar 19, 2023
Closed
@rafsanulhasan
Copy link

@hassanhabib I've just created another PR #84 which allows you to put own dependencies for different brokers.

This time tried to maintain the standard.

@jodendaal jodendaal mentioned this issue Mar 20, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@hassanhabib @rafsanulhasan @jodendaal @BrianLParker