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.

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

Allow an HttpClientFactory to be assigned to OpenAIAPI #103

Merged
merged 1 commit into from Apr 2, 2023

Conversation

babrekel
Copy link
Contributor

@babrekel babrekel commented Mar 31, 2023

This change aims to enable consumers to use IHttpClientFactory and apply resiliency best practices described in this document without a breaking change. Consumers can now implement non-functional requirements such as:

  • Configuring retry policies to the outgoing HTTP requests.
  • Logging and telemetry instrumentation to outgoing HTTP calls.
  • Mocking the HTTP responses for integration testing scenarios

Implementation wise, I opted not to modify the constructor as it takes an optional parameter. Adding another optional parameter is a breaking change, and mixing optional parameters with overloaded constructors is not generally recommended. Effectively seeking to minimize change.

Package wise, the dependency on Microsoft.Extensions.Http is no longer private.

I used Moq in the unit tests to mock an HTTP Client and ensure the right client is used in the right place.

Related issues that could be addressed with this change (or at least allow a workaround):
#102
#100
#91
#41
#28

@OkGoDoIt
Copy link
Owner

OkGoDoIt commented Apr 2, 2023

Thank you @babrekel, this looks good! I appreciate that you found a way to do this without breaking changes.

@OkGoDoIt OkGoDoIt merged commit e585eb2 into OkGoDoIt:master Apr 2, 2023
@naveedahmed1
Copy link

Does it support Typed HttpClient Instance as well?

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

Successfully merging this pull request may close these issues.

None yet

3 participants