From d69bb6de5781aa1765da49fa084e7431191d2863 Mon Sep 17 00:00:00 2001 From: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> Date: Wed, 26 Jun 2024 16:35:27 +0100 Subject: [PATCH 1/4] Bringing OpenAI policy as Utility for OpenAI Connectors --- .../Models/PipelineSynchronousPolicyTests.cs | 56 ------------ .../Connectors.OpenAIV2.csproj | 6 ++ .../Connectors.OpenAIV2/Core/ClientCore.cs | 15 +++- .../Core/Models/AddHeaderRequestPolicy.cs | 23 ----- .../Core/Models/PipelineSynchronousPolicy.cs | 89 ------------------- .../openai/OpenAIUtilities.props | 5 ++ .../Policies/GeneratedActionPipelinePolicy.cs | 45 ++++++++++ .../SemanticKernel.UnitTests.csproj | 2 + .../GenericActionPipelinePolicyTests.cs} | 20 ++--- 9 files changed, 79 insertions(+), 182 deletions(-) delete mode 100644 dotnet/src/Connectors/Connectors.OpenAIV2.UnitTests/Core/Models/PipelineSynchronousPolicyTests.cs delete mode 100644 dotnet/src/Connectors/Connectors.OpenAIV2/Core/Models/AddHeaderRequestPolicy.cs delete mode 100644 dotnet/src/Connectors/Connectors.OpenAIV2/Core/Models/PipelineSynchronousPolicy.cs create mode 100644 dotnet/src/InternalUtilities/openai/OpenAIUtilities.props create mode 100644 dotnet/src/InternalUtilities/openai/Policies/GeneratedActionPipelinePolicy.cs rename dotnet/src/{Connectors/Connectors.OpenAIV2.UnitTests/Core/Models/AddHeaderRequestPolicyTests.cs => SemanticKernel.UnitTests/Utilities/OpenAI/GenericActionPipelinePolicyTests.cs} (54%) diff --git a/dotnet/src/Connectors/Connectors.OpenAIV2.UnitTests/Core/Models/PipelineSynchronousPolicyTests.cs b/dotnet/src/Connectors/Connectors.OpenAIV2.UnitTests/Core/Models/PipelineSynchronousPolicyTests.cs deleted file mode 100644 index cae4b32b4283..000000000000 --- a/dotnet/src/Connectors/Connectors.OpenAIV2.UnitTests/Core/Models/PipelineSynchronousPolicyTests.cs +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using System.ClientModel.Primitives; -using System.Collections.Generic; -using System.Threading.Tasks; -using Microsoft.SemanticKernel.Connectors.OpenAI; -using Xunit; - -namespace SemanticKernel.Connectors.OpenAI.UnitTests.Core.Models; -public class PipelineSynchronousPolicyTests -{ - [Fact] - public async Task ItProcessAsyncWhenSpecializationHasReceivedResponseOverrideShouldCallIt() - { - // Arrange - var first = new MyHttpPipelinePolicyWithoutOverride(); - var last = new MyHttpPipelinePolicyWithOverride(); - - IReadOnlyList policies = [first, last]; - - // Act - await policies[0].ProcessAsync(ClientPipeline.Create().CreateMessage(), policies, 0); - - // Assert - Assert.True(first.CalledProcess); - Assert.True(last.CalledProcess); - Assert.True(last.CalledOnReceivedResponse); - } - - private class MyHttpPipelinePolicyWithoutOverride : PipelineSynchronousPolicy - { - public bool CalledProcess { get; private set; } - - public override void Process(PipelineMessage message, IReadOnlyList pipeline, int currentIndex) - { - this.CalledProcess = true; - base.Process(message, pipeline, currentIndex); - } - - public override ValueTask ProcessAsync(PipelineMessage message, IReadOnlyList pipeline, int currentIndex) - { - this.CalledProcess = true; - return base.ProcessAsync(message, pipeline, currentIndex); - } - } - - private sealed class MyHttpPipelinePolicyWithOverride : MyHttpPipelinePolicyWithoutOverride - { - public bool CalledOnReceivedResponse { get; private set; } - - public override void OnReceivedResponse(PipelineMessage message) - { - this.CalledOnReceivedResponse = true; - } - } -} diff --git a/dotnet/src/Connectors/Connectors.OpenAIV2/Connectors.OpenAIV2.csproj b/dotnet/src/Connectors/Connectors.OpenAIV2/Connectors.OpenAIV2.csproj index b17b14eb91ef..8157312a96a3 100644 --- a/dotnet/src/Connectors/Connectors.OpenAIV2/Connectors.OpenAIV2.csproj +++ b/dotnet/src/Connectors/Connectors.OpenAIV2/Connectors.OpenAIV2.csproj @@ -13,6 +13,7 @@ + @@ -20,6 +21,11 @@ Semantic Kernel connectors for OpenAI and Azure OpenAI. Contains clients for text generation, chat completion, embedding and DALL-E text to image. + + + + + diff --git a/dotnet/src/Connectors/Connectors.OpenAIV2/Core/ClientCore.cs b/dotnet/src/Connectors/Connectors.OpenAIV2/Core/ClientCore.cs index a6be6d20aa46..355000887f51 100644 --- a/dotnet/src/Connectors/Connectors.OpenAIV2/Core/ClientCore.cs +++ b/dotnet/src/Connectors/Connectors.OpenAIV2/Core/ClientCore.cs @@ -116,7 +116,7 @@ internal ClientCore( var options = GetOpenAIClientOptions(httpClient, this.Endpoint); if (!string.IsNullOrWhiteSpace(organizationId)) { - options.AddPolicy(new AddHeaderRequestPolicy("OpenAI-Organization", organizationId!), PipelinePosition.PerCall); + options.AddPolicy(CreateRequestHeaderPolicy("OpenAI-Organization", organizationId!), PipelinePosition.PerCall); this.AddAttribute(ClientCore.OrganizationKey, organizationId); } @@ -184,7 +184,7 @@ private static OpenAIClientOptions GetOpenAIClientOptions(HttpClient? httpClient Endpoint = endpoint }; - options.AddPolicy(new AddHeaderRequestPolicy(HttpHeaderConstant.Names.SemanticKernelVersion, HttpHeaderConstant.Values.GetAssemblyVersion(typeof(ClientCore))), PipelinePosition.PerCall); + options.AddPolicy(CreateRequestHeaderPolicy(HttpHeaderConstant.Names.SemanticKernelVersion, HttpHeaderConstant.Values.GetAssemblyVersion(typeof(ClientCore))), PipelinePosition.PerCall); if (httpClient is not null) { @@ -213,4 +213,15 @@ private static async Task RunRequestAsync(Func> request) throw e.ToHttpOperationException(); } } + + private static GenericActionPipelinePolicy CreateRequestHeaderPolicy(string headerName, string headerValue) + { + return new GenericActionPipelinePolicy((message) => + { + if (message?.Request?.Headers?.TryGetValue(headerName, out string? _) == false) + { + message.Request.Headers.Set(headerName, headerValue); + } + }); + } } diff --git a/dotnet/src/Connectors/Connectors.OpenAIV2/Core/Models/AddHeaderRequestPolicy.cs b/dotnet/src/Connectors/Connectors.OpenAIV2/Core/Models/AddHeaderRequestPolicy.cs deleted file mode 100644 index 2279d639c54e..000000000000 --- a/dotnet/src/Connectors/Connectors.OpenAIV2/Core/Models/AddHeaderRequestPolicy.cs +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -/* Phase 1 -Added from OpenAI v1 with adapted logic to the System.ClientModel abstraction -*/ - -using System.ClientModel.Primitives; - -namespace Microsoft.SemanticKernel.Connectors.OpenAI; - -/// -/// Helper class to inject headers into System ClientModel Http pipeline -/// -internal sealed class AddHeaderRequestPolicy(string headerName, string headerValue) : PipelineSynchronousPolicy -{ - private readonly string _headerName = headerName; - private readonly string _headerValue = headerValue; - - public override void OnSendingRequest(PipelineMessage message) - { - message.Request.Headers.Add(this._headerName, this._headerValue); - } -} diff --git a/dotnet/src/Connectors/Connectors.OpenAIV2/Core/Models/PipelineSynchronousPolicy.cs b/dotnet/src/Connectors/Connectors.OpenAIV2/Core/Models/PipelineSynchronousPolicy.cs deleted file mode 100644 index b7690ead8b7f..000000000000 --- a/dotnet/src/Connectors/Connectors.OpenAIV2/Core/Models/PipelineSynchronousPolicy.cs +++ /dev/null @@ -1,89 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -/* -Phase 1 -As SystemClient model does not have any specialization or extension ATM, introduced this class with the adapted to use System.ClientModel abstractions. -https://github.com/Azure/azure-sdk-for-net/blob/8bd22837639d54acccc820e988747f8d28bbde4a/sdk/core/Azure.Core/src/Pipeline/HttpPipelineSynchronousPolicy.cs -*/ - -using System; -using System.ClientModel.Primitives; -using System.Collections.Generic; -using System.Reflection; -using System.Threading.Tasks; - -namespace Microsoft.SemanticKernel.Connectors.OpenAI; - -/// -/// Represents a that doesn't do any asynchronous or synchronously blocking operations. -/// -internal class PipelineSynchronousPolicy : PipelinePolicy -{ - private static readonly Type[] s_onReceivedResponseParameters = new[] { typeof(PipelineMessage) }; - - private readonly bool _hasOnReceivedResponse = true; - - /// - /// Initializes a new instance of - /// - protected PipelineSynchronousPolicy() - { - var onReceivedResponseMethod = this.GetType().GetMethod(nameof(OnReceivedResponse), BindingFlags.Instance | BindingFlags.Public, null, s_onReceivedResponseParameters, null); - if (onReceivedResponseMethod != null) - { - this._hasOnReceivedResponse = onReceivedResponseMethod.GetBaseDefinition().DeclaringType != onReceivedResponseMethod.DeclaringType; - } - } - - /// - public override void Process(PipelineMessage message, IReadOnlyList pipeline, int currentIndex) - { - this.OnSendingRequest(message); - if (pipeline.Count > currentIndex + 1) - { - // If there are more policies in the pipeline, continue processing - ProcessNext(message, pipeline, currentIndex); - } - this.OnReceivedResponse(message); - } - - /// - public override ValueTask ProcessAsync(PipelineMessage message, IReadOnlyList pipeline, int currentIndex) - { - if (!this._hasOnReceivedResponse) - { - // If OnReceivedResponse was not overridden we can avoid creating a state machine and return the task directly - this.OnSendingRequest(message); - if (pipeline.Count > currentIndex + 1) - { - // If there are more policies in the pipeline, continue processing - return ProcessNextAsync(message, pipeline, currentIndex); - } - } - - return this.InnerProcessAsync(message, pipeline, currentIndex); - } - - private async ValueTask InnerProcessAsync(PipelineMessage message, IReadOnlyList pipeline, int currentIndex) - { - this.OnSendingRequest(message); - if (pipeline.Count > currentIndex + 1) - { - // If there are more policies in the pipeline, continue processing - await ProcessNextAsync(message, pipeline, currentIndex).ConfigureAwait(false); - } - this.OnReceivedResponse(message); - } - - /// - /// Method is invoked before the request is sent. - /// - /// The containing the request. - public virtual void OnSendingRequest(PipelineMessage message) { } - - /// - /// Method is invoked after the response is received. - /// - /// The containing the response. - public virtual void OnReceivedResponse(PipelineMessage message) { } -} diff --git a/dotnet/src/InternalUtilities/openai/OpenAIUtilities.props b/dotnet/src/InternalUtilities/openai/OpenAIUtilities.props new file mode 100644 index 000000000000..e865b7fe40e9 --- /dev/null +++ b/dotnet/src/InternalUtilities/openai/OpenAIUtilities.props @@ -0,0 +1,5 @@ + + + + + \ No newline at end of file diff --git a/dotnet/src/InternalUtilities/openai/Policies/GeneratedActionPipelinePolicy.cs b/dotnet/src/InternalUtilities/openai/Policies/GeneratedActionPipelinePolicy.cs new file mode 100644 index 000000000000..931f12957965 --- /dev/null +++ b/dotnet/src/InternalUtilities/openai/Policies/GeneratedActionPipelinePolicy.cs @@ -0,0 +1,45 @@ +// Copyright (c) Microsoft. All rights reserved. + +/* Phase 03 +Adapted from OpenAI SDK original policy with warning updates. + +Original file: https://github.com/openai/openai-dotnet/blob/0b97311f58dfb28bd883d990f68d548da040a807/src/Utility/GenericActionPipelinePolicy.cs#L8 +*/ + +using System; +using System.ClientModel.Primitives; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; + +/// +/// Generic action pipeline policy for processing messages. +/// +[ExcludeFromCodeCoverage] +internal sealed class GenericActionPipelinePolicy : PipelinePolicy +{ + private readonly Action _processMessageAction; + + internal GenericActionPipelinePolicy(Action processMessageAction) + { + this._processMessageAction = processMessageAction; + } + + public override void Process(PipelineMessage message, IReadOnlyList pipeline, int currentIndex) + { + this._processMessageAction(message); + if (currentIndex < pipeline.Count - 1) + { + pipeline[currentIndex + 1].Process(message, pipeline, currentIndex + 1); + } + } + + public override async ValueTask ProcessAsync(PipelineMessage message, IReadOnlyList pipeline, int currentIndex) + { + this._processMessageAction(message); + if (currentIndex < pipeline.Count - 1) + { + await pipeline[currentIndex + 1].ProcessAsync(message, pipeline, currentIndex + 1).ConfigureAwait(false); + } + } +} diff --git a/dotnet/src/SemanticKernel.UnitTests/SemanticKernel.UnitTests.csproj b/dotnet/src/SemanticKernel.UnitTests/SemanticKernel.UnitTests.csproj index e929fe1ca82f..3cbaf6b60797 100644 --- a/dotnet/src/SemanticKernel.UnitTests/SemanticKernel.UnitTests.csproj +++ b/dotnet/src/SemanticKernel.UnitTests/SemanticKernel.UnitTests.csproj @@ -28,6 +28,7 @@ + @@ -38,6 +39,7 @@ + diff --git a/dotnet/src/Connectors/Connectors.OpenAIV2.UnitTests/Core/Models/AddHeaderRequestPolicyTests.cs b/dotnet/src/SemanticKernel.UnitTests/Utilities/OpenAI/GenericActionPipelinePolicyTests.cs similarity index 54% rename from dotnet/src/Connectors/Connectors.OpenAIV2.UnitTests/Core/Models/AddHeaderRequestPolicyTests.cs rename to dotnet/src/SemanticKernel.UnitTests/Utilities/OpenAI/GenericActionPipelinePolicyTests.cs index 83ec6a20568d..e0296b253858 100644 --- a/dotnet/src/Connectors/Connectors.OpenAIV2.UnitTests/Core/Models/AddHeaderRequestPolicyTests.cs +++ b/dotnet/src/SemanticKernel.UnitTests/Utilities/OpenAI/GenericActionPipelinePolicyTests.cs @@ -1,39 +1,35 @@ // Copyright (c) Microsoft. All rights reserved. -using System.ClientModel.Primitives; -using Microsoft.SemanticKernel.Connectors.OpenAI; using Xunit; +using System.ClientModel.Primitives; -namespace SemanticKernel.Connectors.OpenAI.UnitTests.Core.Models; +namespace SemanticKernel.UnitTests.Utilities.OpenAI; -public class AddHeaderRequestPolicyTests +public class GenericActionPipelinePolicyTests { [Fact] public void ItCanBeInstantiated() { - // Arrange - var headerName = "headerName"; - var headerValue = "headerValue"; - // Act - var addHeaderRequestPolicy = new AddHeaderRequestPolicy(headerName, headerValue); + var addHeaderRequestPolicy = new GenericActionPipelinePolicy((message) => { }); // Assert Assert.NotNull(addHeaderRequestPolicy); } [Fact] - public void ItOnSendingRequestAddsHeaderToRequest() + public void ItProcessAddsHeaderToRequest() { // Arrange var headerName = "headerName"; var headerValue = "headerValue"; - var addHeaderRequestPolicy = new AddHeaderRequestPolicy(headerName, headerValue); + var sut = new GenericActionPipelinePolicy((message) => { message.Request.Headers.Add(headerName, headerValue); }); + var pipeline = ClientPipeline.Create(); var message = pipeline.CreateMessage(); // Act - addHeaderRequestPolicy.OnSendingRequest(message); + sut.Process(message, [sut], 0); // Assert message.Request.Headers.TryGetValue(headerName, out var value); From ab4155694f08a36adf4addc9b1c9a55f902d0538 Mon Sep 17 00:00:00 2001 From: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> Date: Wed, 26 Jun 2024 16:42:37 +0100 Subject: [PATCH 2/4] Warning fix --- .../Utilities/OpenAI/GenericActionPipelinePolicyTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/SemanticKernel.UnitTests/Utilities/OpenAI/GenericActionPipelinePolicyTests.cs b/dotnet/src/SemanticKernel.UnitTests/Utilities/OpenAI/GenericActionPipelinePolicyTests.cs index e0296b253858..ca36f300b1c2 100644 --- a/dotnet/src/SemanticKernel.UnitTests/Utilities/OpenAI/GenericActionPipelinePolicyTests.cs +++ b/dotnet/src/SemanticKernel.UnitTests/Utilities/OpenAI/GenericActionPipelinePolicyTests.cs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. -using Xunit; using System.ClientModel.Primitives; +using Xunit; namespace SemanticKernel.UnitTests.Utilities.OpenAI; From 29daf8bd2b2e7b6f81f1a499eea609ed11623d6d Mon Sep 17 00:00:00 2001 From: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> Date: Wed, 26 Jun 2024 17:09:56 +0100 Subject: [PATCH 3/4] Removing unnecessary configuration --- .../Connectors.OpenAIV2/Connectors.OpenAIV2.csproj | 5 ----- 1 file changed, 5 deletions(-) diff --git a/dotnet/src/Connectors/Connectors.OpenAIV2/Connectors.OpenAIV2.csproj b/dotnet/src/Connectors/Connectors.OpenAIV2/Connectors.OpenAIV2.csproj index 8157312a96a3..22f364461818 100644 --- a/dotnet/src/Connectors/Connectors.OpenAIV2/Connectors.OpenAIV2.csproj +++ b/dotnet/src/Connectors/Connectors.OpenAIV2/Connectors.OpenAIV2.csproj @@ -21,11 +21,6 @@ Semantic Kernel connectors for OpenAI and Azure OpenAI. Contains clients for text generation, chat completion, embedding and DALL-E text to image. - - - - - From d240c15077a325b655e5f94fdfc6bc829816cef0 Mon Sep 17 00:00:00 2001 From: Roger Barreto <19890735+RogerBarreto@users.noreply.github.com> Date: Wed, 26 Jun 2024 22:49:50 +0100 Subject: [PATCH 4/4] Add utilities to solution --- dotnet/SK-dotnet.sln | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dotnet/SK-dotnet.sln b/dotnet/SK-dotnet.sln index 326a35a79ff7..6da6c33ec47a 100644 --- a/dotnet/SK-dotnet.sln +++ b/dotnet/SK-dotnet.sln @@ -327,6 +327,16 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Connectors.AzureOpenAI", "s EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Connectors.AzureOpenAI.UnitTests", "src\Connectors\Connectors.AzureOpenAI.UnitTests\Connectors.AzureOpenAI.UnitTests.csproj", "{DB219924-208B-4CDD-8796-EE424689901E}" EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "openai", "openai", "{2E79AD99-632F-411F-B3A5-1BAF3F5F89AB}" + ProjectSection(SolutionItems) = preProject + src\InternalUtilities\openai\OpenAIUtilities.props = src\InternalUtilities\openai\OpenAIUtilities.props + EndProjectSection +EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Policies", "Policies", "{7308EF7D-5F9A-47B2-A62F-0898603262A8}" + ProjectSection(SolutionItems) = preProject + src\InternalUtilities\openai\Policies\GeneratedActionPipelinePolicy.cs = src\InternalUtilities\openai\Policies\GeneratedActionPipelinePolicy.cs + EndProjectSection +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -932,6 +942,8 @@ Global {FDEB4884-89B9-4656-80A0-57C7464490F7} = {831DDCA2-7D2C-4C31-80DB-6BDB3E1F7AE0} {6744272E-8326-48CE-9A3F-6BE227A5E777} = {1B4CBDE0-10C2-4E7D-9CD0-FE7586C96ED1} {DB219924-208B-4CDD-8796-EE424689901E} = {1B4CBDE0-10C2-4E7D-9CD0-FE7586C96ED1} + {2E79AD99-632F-411F-B3A5-1BAF3F5F89AB} = {4D3DAE63-41C6-4E1C-A35A-E77BDFC40675} + {7308EF7D-5F9A-47B2-A62F-0898603262A8} = {2E79AD99-632F-411F-B3A5-1BAF3F5F89AB} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {FBDC56A3-86AD-4323-AA0F-201E59123B83}