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

This service provider doesn't support keyed services. #14983

Closed
MikeAlhayek opened this issue Jan 3, 2024 · 8 comments · Fixed by #14998
Closed

This service provider doesn't support keyed services. #14983

MikeAlhayek opened this issue Jan 3, 2024 · 8 comments · Fixed by #14998
Assignees
Labels
Milestone

Comments

@MikeAlhayek
Copy link
Member

MikeAlhayek commented Jan 3, 2024

Describe the bug

It looks like we still don't fully support keyed service.

I tried to register services using implementation factory like this

services.AddKeyedScoped<OpenAIClient>("SomeKey", (sp, obj) =>
{
    var openAIOptions = sp.GetRequiredService<IOptions<OpenAIOptions >>().Value;

    if (string.IsNullOrEmpty(openAIOptions.Host))
    {
        throw new InvalidOperationException($"{nameof(openAIOptions.Host)} is not configured.");
    }

    if (string.IsNullOrEmpty(openAIOptions.ApiKey))
    {
        throw new InvalidOperationException($"{nameof(openAIOptions.ApiKey)} is not configured.");
    }

    return new OpenAIClient(new Uri(openAIOptions.Host), new AzureKeyCredential(openAIOptions.ApiKey), GetOpenAIClientOptions(openAIOptions.ApiVersion));
});

then in the controller, I tried to resolve the service like this

public AdminController([FromKeyedServices("SomeKey")] OpenAIClient client)
{
    _client = client;
}

That gave an the following exception

InvalidOperationException: This service provider doesn't support keyed services.

Expected behavior

I expected that the controller will be instantiated with an instance of OpenAIClient what matched "SomeKey" key.

@hishamco
Copy link
Member

hishamco commented Jan 4, 2024

@MikeAlhayek do you plan to submit a PR or shall I do it? coz I already faced one issue in the past

@MikeAlhayek
Copy link
Member Author

@hishamco I have too many things going on that this moment. If you have the time, a PR form you would be amazing :)

@hishamco hishamco self-assigned this Jan 5, 2024
@hishamco
Copy link
Member

hishamco commented Jan 5, 2024

@MikeAlhayek I just finished a unit test to ensure that your case has been worked fine

@hishamco
Copy link
Member

hishamco commented Jan 5, 2024

Here is the full unit test code

[Fact]
public void GetKeyedService_RegisteredUsingImplementationFactory()
{
    // Arrange
    var services = new ServiceCollection();

    services.Configure<OpenAIOptions>(options =>
    {
        options.ApiKey = "123";
        options.ApiVersion = "2020-10-10";
        options.Host = "https://api.openai.com";
    });

    services.AddKeyedScoped("SomeKey", (sp, obj) =>
    {
        var openAIOptions = sp.GetRequiredService<IOptions<OpenAIOptions>>().Value;

        if (string.IsNullOrEmpty(openAIOptions.Host))
        {
            throw new InvalidOperationException($"{nameof(openAIOptions.Host)} is not configured.");
        }

        if (string.IsNullOrEmpty(openAIOptions.ApiKey))
        {
            throw new InvalidOperationException($"{nameof(openAIOptions.ApiKey)} is not configured.");
        }

        return new OpenAIClient(new Uri(openAIOptions.Host), new AzureKeyCredential(openAIOptions.ApiKey), GetOpenAIClientOptions(openAIOptions.ApiVersion));
    });

    services.AddScoped<AdminController>();

    var serviceProvider = services.BuildServiceProvider();

    // Act
    var controller = serviceProvider.GetRequiredService<AdminController>();

    // Assert
    Assert.IsType<OpenAIClient>(controller.Client);
}

private static OpenAIOptions GetOpenAIClientOptions(string apiVersion) => new();
public class OpenAIClient
{
    public OpenAIClient(Uri uri, AzureKeyCredential credential, OpenAIOptions options)
    {
        
    }
}
public class OpenAIOptions
{
    public string ApiKey { get; set; }

    public string ApiVersion { get; set; }

    public string Host { get; set; }
}
public class AdminController([FromKeyedServices("SomeKey")] KeyedServiceTests.OpenAIClient client)
{
    public OpenAIClient Client { get; } = client;
}

Please make it fail if I miss something in your case

@hishamco
Copy link
Member

hishamco commented Jan 5, 2024

If no action needed please close

@MikeAlhayek
Copy link
Member Author

@hishamco the reason why your test case passed is because you are testing a standard asp.net core ServiceCollection which supports keyed services. In OC, we use ShellScopeServices which currently does not support keyed services.

The good news is that I fixed this issue using PR #14998

@hishamco
Copy link
Member

hishamco commented Jan 5, 2024

I'm unsure why we did it using normal ASP.NET Core ServiceCollection or creating a wrapper around it.

@MikeAlhayek
Copy link
Member Author

@hishamco not sure either. I kinds feel useless to me. But I am sure it was added for a valid reason. If you care, try removing it and see if everything works fine including background processes. It's worth looking into it.

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

Successfully merging a pull request may close this issue.

2 participants