-
Notifications
You must be signed in to change notification settings - Fork 4k
.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
Changes from 6 commits
ea3355e
161f1f6
ea725e5
8a3fba9
6b87674
1a06eae
06dece3
650071f
2b66447
54e07c2
265c732
7cc6403
0911792
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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; } | ||
|
||
/// <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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In current extension methods this parameter has a name |
||
|
||
/// <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; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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"); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
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. ForAPITypes
- I'm not sure we need it at all.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.