Support Dependency Injection

[KeithHenry]

Breaking change: switch from inline instantiation to using .NET's dependency injection.

Current usage will no longer work:

var api = new OpenAI_API.OpenAIAPI("YOUR_API_KEY");

The OpenAIAPI constructor is now internal, and takes a client generated by an HttpClientFactory (fixes #41)

Instead in your startup register the service:

services.AddOpenAIService(config);

This will inject IOpenAI, which you can access with [FromServices] or serviceProvider.GetService<IOpenAI>()

Also fixes #42 and #43

[OkGoDoIt]

Wow @KeithHenry, thank you for putting the effort into implementing this, it's no small feat! 🤩

I have concerns about adding DI, both compatibility and ease-of use. I notice the PR eliminates support for .NET Standard and therefore the .NET Framework. It also adds several Nuget Package dependencies. It feels optimized for Asp.net rather than broad usage. Is DI widely used outside of the asp.net use case? I want to ensure this still works for Unity, Mobile, WinForms, etc.

Can you include updated readme examples? I would want to ensure the simple use cases are still easy to achieve, especially for the majority of .Net developers who may be unfamiliar with DI.

I have to admit I'm not that familiar with DI in .Net, so I may be a bit biased against it, but I'm willing to be convinced. I just want to make sure we aren't adding complications for 90% of casual users to accommodate the use cases of 10% of enterprise users.

[gotmike]

it would be nice if we could support both, but i think DI is the way things are going regardless.

i can't speak to all these platforms, but i have used it in console apps.

i know this kindof pushes the legacy users out though. would be great if both options were available through separate endpoints.

https://learn.microsoft.com/en-us/dotnet/core/extensions/dependency-injection

[KeithHenry]

@OkGoDoIt answers to your points

I notice the PR eliminates support for .NET Standard and therefore the .NET Framework. It also adds several Nuget Package dependencies.

Yup, you could probably support net-standard2.1 but I'm not sure what the alternate dependencies would be. I had time to upgrade it to current .NET, I think finding that balance between old versions is possible, but more than I have time to contribute right now. This PR is a starting point for DI support, it isn't ready to merge.

The dependencies it adds are ones everyone using .NET5 or above already has

It feels optimized for Asp.net rather than broad usage. Is DI widely used outside of the asp.net use case? I want to ensure this still works for Unity, Mobile, WinForms, etc.

Since .NET 5 it's been the default for all apps.

I just want to make sure we aren't adding complications for 90% of casual users to accommodate the use cases of 10% of enterprise users.

I get that, but if I was doing this in my own time I'd just be using curl (or something simple like it) to get Open AI requests, I probably wouldn't be on NuGet looking for a package to do it.

However, a casual user starting a new .NET app (console, web, whatever) in Visual Studio 2022 Community or with the dotnet command line will get DI as their boilerplate code.

[Baklap4]

Yup, you could probably support net-standard2.1 but I'm not sure what the alternate dependencies would be. I had time to upgrade it to current .NET, I think finding that balance between old versions is possible, but more than I have time to contribute right now. This PR is a starting point for DI support, it isn't ready to merge.

I think the introduced packages are all compatible with .netstandard20 according to nuget.org So there's no need to bump the targetframework from netstandard2.0 to net7.0?

It'd be nice if we can upgrade to .net7.0 since this allows us to use the latest C# language features, but this comes at the cost of dropping netframework support. Another possibility is to have the packages released for multiple targetframework; aka .net7.0 AND netstandard 2.0

[bdemarzo]

I love the DI support, but it probably should be a separate package that adds DI support on top of the core.

[KeithHenry]

I love the DI support, but it probably should be a separate package that adds DI support on top of the core.

This library has a severe bug #41 - this PR fixes that, but can't keep backwards support.

There's a possible workaround, but it would still make the DI version the main one.