Skip to content

.Net: Adding Config classes for use with IKernelBuilder and IServiceCollection #6283

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

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using Azure.Core;

namespace Microsoft.SemanticKernel;

public class AzureOpenAIConfig
{
private TokenCredential? _tokenCredential;

public enum AuthTypes
{
Unknown = -1,
AzureIdentity,
APIKey,
ManualTokenCredential,
}

public enum APITypes
{
Unknown = -1,
TextCompletion,
ChatCompletion,
ImageGeneration,
EmbeddingGeneration,
}

/// <summary>
/// OpenAI API type, e.g. text completion, chat completion, image generation, etc.
/// </summary>
public APITypes APIType { get; set; } = APITypes.ChatCompletion;

/// <summary>
/// Azure authentication type.
/// </summary>
public AuthTypes Auth { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As already mentioned, it would be nice to avoid using these enums. For AuthTypes - define specific classes. For APITypes - I'm not sure we need it at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my comment below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As requested I created specific classes for the configuration and removed the enums. Please take a look.


/// <summary>
/// Azure OpenAI endpoint URL.
/// </summary>
public string Endpoint { get; set; } = string.Empty;

/// <summary>
/// Azure OpenAI deployment name.
/// </summary>
public string Deployment { get; set; } = string.Empty;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In current extension methods this parameter has a name deploymentName. I would keep the same naming in config classes and current extension methods for consistency.


/// <summary>
/// API key, required if Auth == APIKey.
/// </summary>
public string APIKey { get; set; } = string.Empty;

/// <summary>
/// The number of dimensions output embeddings should have.
/// Only supported in "text-embedding-3" and later models developed with
/// MRL, see https://arxiv.org/abs/2205.13147
/// </summary>
public int? EmbeddingDimensions { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property is not applicable for chat completion endpoints. Can we organize configuration model hierarchy by modality type (e.g. chat completion, text generation, audio) instead of auth type? Then, each model could contain overload constructors which accept either apiKey or TokenCredential.

This should be more scalable, as I expect more different modalities supported with more configuration properties comparing to auth types.


/// <summary>
/// Set credentials manually from code.
/// </summary>
/// <param name="credential">Token credentials</param>
public void SetCredential(TokenCredential credential)
{
this.Auth = AuthTypes.ManualTokenCredential;
this._tokenCredential = credential;
}

/// <summary>
/// Fetch the credentials passed manually from code.
/// </summary>
public TokenCredential GetTokenCredential()
{
return this._tokenCredential
?? throw new ConfigurationException($"Azure OpenAI: {nameof(this._tokenCredential)} not defined");
}

/// <summary>
/// Verify that the current state is valid.
/// </summary>
public void Validate()
{
if (this.Auth == AuthTypes.Unknown)
{
throw new ConfigurationException($"Azure OpenAI: {nameof(this.Auth)} (authentication type) is not defined");
}

if (this.Auth == AuthTypes.APIKey && string.IsNullOrWhiteSpace(this.APIKey))
{
throw new ConfigurationException($"Azure OpenAI: {nameof(this.APIKey)} is empty");
}

if (string.IsNullOrWhiteSpace(this.Endpoint))
{
throw new ConfigurationException($"Azure OpenAI: {nameof(this.Endpoint)} is empty");
}

if (!this.Endpoint.StartsWith("https://", StringComparison.OrdinalIgnoreCase))
{
throw new ConfigurationException($"Azure OpenAI: {nameof(this.Endpoint)} must start with https://");
}

if (string.IsNullOrWhiteSpace(this.Deployment))
{
throw new ConfigurationException($"Azure OpenAI: {nameof(this.Deployment)} (deployment name) is empty");
}

if (this.EmbeddingDimensions is < 1)
{
throw new ConfigurationException($"Azure OpenAI: {nameof(this.EmbeddingDimensions)} cannot be less than 1");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// Copyright (c) Microsoft. All rights reserved.

using System.Text.Json.Serialization;

namespace Microsoft.SemanticKernel;

/// <summary>
/// OpenAI settings.
/// </summary>
public class OpenAIConfig
{
[JsonConverter(typeof(JsonStringEnumConverter))]
public enum TextGenerationTypes
{
Auto = 0,
TextCompletion,
Chat,
}

/// <summary>
/// Model used for text generation. Chat models can be used too.
/// </summary>
public string TextModel { get; set; } = string.Empty;

/// <summary>
/// The type of OpenAI completion to use, either Text (legacy) or Chat.
/// When using Auto, the client uses OpenAI model names to detect the correct protocol.
/// Most OpenAI models use Chat. If you're using a non-OpenAI model, you might want to set this manually.
/// </summary>
public TextGenerationTypes TextGenerationType { get; set; } = TextGenerationTypes.Auto;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find a place where this property and enum are used. Would it be possible to remove it?


/// <summary>
/// Model used to embedding generation.
/// </summary>
public string EmbeddingModel { get; set; } = string.Empty;

/// <summary>
/// OpenAI HTTP endpoint. You may need to override this to work with
/// OpenAI compatible services like LM Studio.
/// </summary>
public string Endpoint { get; set; } = "https://api.openai.com/v1";

/// <summary>
/// OpenAI API key.
/// </summary>
public string APIKey { get; set; } = string.Empty;

/// <summary>
/// Optional OpenAI Organization ID.
/// </summary>
public string? OrgId { get; set; } = string.Empty;

/// <summary>
/// The number of dimensions output embeddings should have.
/// Only supported in "text-embedding-3" and later models developed with
/// MRL, see https://arxiv.org/abs/2205.13147
/// </summary>
public int? EmbeddingDimensions { get; set; }

/// <summary>
/// Verify that the current state is valid.
/// </summary>
public void Validate()
{
if (string.IsNullOrWhiteSpace(this.APIKey))
{
throw new ConfigurationException($"OpenAI: {nameof(this.APIKey)} is empty");
}

if (this.EmbeddingDimensions is < 1)
{
throw new ConfigurationException($"OpenAI: {nameof(this.EmbeddingDimensions)} cannot be less than 1");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,6 @@

<ItemGroup>
<PackageReference Include="Azure.AI.OpenAI" />
<PackageReference Include="Azure.Identity" />
</ItemGroup>
</Project>
Loading
Loading