Providing OpenAIOptions not really works about forge.openai HOT 8 OPEN

Still great thanks for the library - we already integrated and it works.

My only concern was that it's not really friendly for a person who doesn't do deep dive in DI :-)

from forge.openai.

Hello,

Thanks for the feedback.

1, Need to understand when and where we can use the methods and what are our responsibilities. This method is just for the very simple scenarios, when a developer wants to use it without DI just with a provided options AND when the instance is global with same lifecycle of the running application. In that case, it does not necessary to dispose the internal ServiceProvider.
You can find examples in the Playgrounds, project named "MultipleApiKeyUsage" project.

2, I am trying to follow the description what you wrote, but I do not understand the workflow. After you registered the servuce the "AddForgeOpenAIAsTransient", than why do you call the CreateService method? This method designed to use when you manually create your own IServiceProvider instance and you are not in a hosting model envronment. If you create your own ServiceCollection -> register the services an your own requirements -> build your ServiceProvider and ask an instance of IOpenAIService from your provider instance. This is what exactly happen in the "CreateService(OpenAIOptions options)".

In a hosting model environment, instances will be injected, for example in blazor, or also use the IServiceProvider of the host.

Btw, the method "CreateService(OpenAIOptions options, IServiceProvider serviceProvider, IProviderEndpointService provider)" is wrong, the third option does not necessary, it was left after one of my refactoring. I will remove the third option in the next release and it was acquired from the given "serviceProvider".

3, You are right, it is a singleton. It is mostly okay for the apps, but I know, it is not good for every scenario. But feel free to copy/paste my extension methods and create your own ones which are fit for your requirements. That was the reason why I was not set the class visibilities to "internal" as normal in other libraries to give freedom for the SDK users to configure thier environment as they wish. So just simply copy/paste this "AddForgeOpenAIAsTransient" into your own extension class, give it a name and change registration modes. For example from singleton to transient.

Once again, thank you for your feedback. I would like to make the SDK as flexible as possible, sometimes this lead to misunderstandings and of course, I cannot provide service registrations for every cases in the universe :)

Happy coding!

from forge.openai.

My point is method registrations *AsScoped will not do what you want in all cases.

For example AddForgeOpenAIAsScoped adds scoped OpenAIOptions but OpenAIProviderEndpointService is still registered as a singleton. Therefore it will not get OpenAIOptions from scope but instead will get a global one.

It doesn't matter how many scopes you create because OpenAIProviderEndpointService is a singleton.

The same issue with ApiHttpLoggerService. Singleton but depends on the scoped OpenAIOptions.

If you want *AsScoped to get scoped options you need to register all services that depend on OpenAIOptions as scoped.

As for *AsTransient this is even worse. Because in all services you directly use

  _apiHttpService = serviceProvider.GetRequiredService<IApiHttpService>();

Which means that ApiHttpService will not get the options you provided. Generally direct usage of GetRequiredService is an antipattern called service locator. If you simply require this service via constructor it will be much better because in this case one can inject:

Func<OpenAIOptions, IOpenAIService> 

And provide transient options into each and every location

from forge.openai.

I tried to write an example but it failed unfortunately because of other reasons.
Note that ambigous constructors issue.

I managed to run it in the end in my system cause I'm using Autofac and can actually resolve ambiguity there.

// See https://aka.ms/new-console-template for more information
using Forge.OpenAI;
using Forge.OpenAI.Authentication;
using Forge.OpenAI.Interfaces.Services;
using Forge.OpenAI.Settings;
using Microsoft.Extensions.DependencyInjection;

ServiceCollection services = new ServiceCollection();
services.AddForgeOpenAIAsScoped();

// Need to do it because OpenAIOptions is not anyhow registered
// services.AddScoped<OpenAIOptions>();

using var provider = services.BuildServiceProvider();

var scope1 = provider.CreateScope();
var opt1 = scope1.ServiceProvider.GetRequiredService<OpenAIOptions>();
opt1.AuthenticationInfo = new AuthenticationInfo()
{
    ApiKey = "first_key"
};
// Fails - ambigous constructors
var service1 = scope1.ServiceProvider.GetRequiredService<IOpenAIService>();

var scope2 = provider.CreateScope();
var opt2 = scope2.ServiceProvider.GetRequiredService<OpenAIOptions>();
opt2AuthenticationInfo = new AuthenticationInfo()
{
    ApiKey = "second_key"
};
// Fails - ambigous constructors
var service2 = scope2.ServiceProvider.GetRequiredService<IOpenAIService>();

from forge.openai.

I got you. I am going to fix this in the next release soon.

from forge.openai.

My original plan was also to use the library without DI as well. Not just because a developer does not have experience with, but because it is not possible to use in the target environment. Maybe I need to improve somehow this question and/or create more examples in the Playground

from forge.openai.

I am just curious, what is your project about? :) I have never got feedback yet!

from forge.openai.

Please check v1.4.0, a couple of improvements made, supporting v2 APIs, DI changes, etc.

from forge.openai.