-
Notifications
You must be signed in to change notification settings - Fork 1
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
Investigation: Azure and unbranded clients #7
base: azoai-generate-clean
Are you sure you want to change the base?
Conversation
EndProject | ||
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "AzureOpenAI", "azoai\AzureOpenAI\AzureOpenAI.csproj", "{FB61EB83-7171-48F4-9423-E64B99BDAA5A}" | ||
EndProject | ||
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.ClientModel", "..\..\..\..\azure-sdk-for-net\sdk\core\System.ClientModel\src\System.ClientModel.csproj", "{8364F37B-2B3D-4CAA-8F20-5B808E19C6CE}" |
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.
Points to the version of ClientModel in this PR: Prototype of JsonModel abstract class
// TODO: Do something with output | ||
} | ||
|
||
void CallAzureService() |
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.
Illustrates application call to Azure OpenAI service.
|
||
namespace AzureOpenAI; | ||
|
||
internal class AzureChatClient : Chat |
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.
Sub-client is internal.
Argument.AssertNotNull(createChatCompletionRequest, nameof(createChatCompletionRequest)); | ||
|
||
using BinaryContent content = createChatCompletionRequest.ToBinaryContent(); | ||
ClientResult result = await CreateChatCompletionAsync(createChatCompletionRequest.Model.ToString(), content, context: default).ConfigureAwait(false); |
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.
Overrides convenience method to call Azure-specific protocol method.
|
||
using BinaryContent content = createChatCompletionRequest.ToBinaryContent(); | ||
ClientResult result = await CreateChatCompletionAsync(createChatCompletionRequest.Model.ToString(), content, context: default).ConfigureAwait(false); | ||
var value = ModelReaderWriter.Read<CreateChatCompletionResponse>(result.GetRawResponse().Content)!; |
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.
Uses ModelReaderWriter to create response model.
return ClientResult.FromValue(value, result.GetRawResponse()); | ||
} | ||
|
||
public override Task<ClientResult> CreateChatCompletionAsync(BinaryContent content, RequestOptions context = null) |
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.
Overrides non-functional protocol method and throws from it.
return ClientResult.FromResponse(Pipeline.ProcessMessage(message, context)); | ||
} | ||
|
||
public async Task<ClientResult> CreateChatCompletionAsync(string model, BinaryContent content, RequestOptions context = null) |
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.
Overloads protocol method with Azure-specific protocol signature.
return ClientResult.FromResponse(await Pipeline.ProcessMessageAsync(message, context).ConfigureAwait(false)); | ||
} | ||
|
||
private PipelineMessage CreateCreateChatCompletionRequest(string model, BinaryContent content, RequestOptions context) |
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.
Implements private request-creation helper that creates request with Azure-specific URI and path parameters. Note here that request content has already been serialized by the model types.
{ | ||
Argument.AssertNotNull(createChatCompletionRequest, nameof(createChatCompletionRequest)); | ||
|
||
using BinaryContent content = createChatCompletionRequest.ToBinaryContent(); |
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.
Extension method on third-party type creates BinaryContent instance that writes content via internal Azure model wrapping third-party type.
_apiKeyCredential = credential; | ||
} | ||
|
||
public override Chat GetChatClient() |
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.
Overrides subclient factory method to return Azure-specific subclient.
public static class ChatClientExtensions | ||
{ | ||
// Expose overloads of protocol methods as extensions to the base client | ||
public static ClientResult CreateChatCompletion(this Chat client, string model, BinaryContent content, RequestOptions options = default) |
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.
Expose overloads of protocol methods as extensions to the base client.
namespace AzureOpenAI.Models; | ||
|
||
[PersistableModelProxy(typeof(UnknownAzureChatExtensionConfiguration))] | ||
public partial class AzureChatExtensionConfiguration : IJsonModel<AzureChatExtensionConfiguration> |
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.
Models specific to Azure are generated in standard way.
|
||
namespace AzureOpenAI.Models; | ||
|
||
internal class AzureCreateChatCompletionRequest : IJsonModel<AzureCreateChatCompletionRequest> |
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.
Internal model wraps third-party model to enable serializing model with Azure-added properties.
writer.WriteStartObject(); | ||
|
||
// Note that we write the base model differently in this context | ||
((IJsonModel<object>)_request).Write(writer, new ModelReaderWriterOptions("W*")); |
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.
Writes third-party model using "W*"/partial format.
|
||
namespace AzureOpenAI; | ||
|
||
internal class JsonModelList<TModel> : List<TModel>, IJsonModel<JsonModelList<TModel>> |
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.
Proposal for emitted type to simplify reading/writing collections.
|
||
public static class CreateChatCompletionRequestExtensions | ||
{ | ||
public static IList<AzureChatExtensionConfiguration> GetDataSources(this CreateChatCompletionRequest request) |
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.
To be replaced with DataSources
extension property in the future.
|
||
JsonModelList<AzureChatExtensionConfiguration> dataSources; | ||
|
||
if (model.AdditionalProperties.TryGetValue("data_sources", out object? value)) |
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.
Retrieves value from AdditionalProperties
dictionary, or creates it new and adds it to the dictionary.
|
||
namespace AzureOpenAI.Models; | ||
|
||
public partial class AzureChatExtensionDataSourceResponseCitation : IJsonModel<AzureChatExtensionDataSourceResponseCitation> |
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.
public output models specific to Azure are generated normally.
public static class ChatCompletionResponseMessageExtensions | ||
{ | ||
// Output property | ||
public static AzureChatExtensionsMessageContext? GetAzureExtensionsContext(this ChatCompletionResponseMessage message) |
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.
To be replaced with extension property AzureExtensionsContext
in the future.
// It's either deserialized already or not. Find out now. | ||
// This should work for all cases because we know the contract regarding | ||
// BinaryData values. | ||
if (value is BinaryData serializedValue) |
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.
Deserialized value if needed and returns from extension property.
@@ -33,7 +33,7 @@ protected Chat() | |||
/// <param name="pipeline"> The HTTP pipeline for sending and receiving REST requests and responses. </param> | |||
/// <param name="keyCredential"> The key credential to copy. </param> | |||
/// <param name="endpoint"> OpenAI Endpoint. </param> | |||
internal Chat(ClientPipeline pipeline, ApiKeyCredential keyCredential, Uri endpoint) | |||
protected internal Chat(ClientPipeline pipeline, ApiKeyCredential keyCredential, Uri endpoint) |
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.
Third-party subclient constructor becomes protected internal.
@@ -10,9 +10,9 @@ | |||
|
|||
namespace OpenAI.Models | |||
{ | |||
public partial class ChatCompletionResponseMessage : IJsonModel<ChatCompletionResponseMessage> | |||
public partial class ChatCompletionResponseMessage : JsonModel<ChatCompletionResponseMessage> |
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.
Inherits from JsonModel
abstract class in System.ClientModel to add AdditionalProperties
dictionary.
} | ||
#endif | ||
} | ||
WriteUnknownProperties(writer, options); |
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.
Writes unknown properties through protected method on the base type.
/// </list> | ||
/// </para> | ||
/// </summary> | ||
private IDictionary<string, BinaryData> _serializedAdditionalRawData; |
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.
Serialized values are now handled through AdditionalProperties dictionary on base JsonModel
type.
{ | ||
throw new FormatException($"The model {nameof(CreateChatCompletionRequest)} does not support writing '{format}' format."); | ||
} | ||
|
||
writer.WriteStartObject(); | ||
if (format == "J") |
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.
Allow writing partial content instead of full object to enable writing content with additions in the extended type.
This PR explores letting extending models write their own ToBinaryContent methods