Http consistency: Pinecone (microsoft#1262)

### Description

This PR introduces the following changes:

1. The PineconeClient class accepts a custom HTTP client, allowing the
hosting application/client code to provide their own instances.

2. A new extension method, `WithPineconeMemoryStore`, has been added to
the `KernelBuilder` class. This method simplifies the registration of
the Pinecone memory store, aligns the registration process with other
connector registrations, and enforces the usage of an HTTP client and
retry handlers configured in the SK SDK.

3. The global static `HttpHandlers` class is removed because it goes
against the agreed approach for the SK SDK's HTTP stack that assumes the
usage of a custom HTTP client for fine-tuning instead of relying on
global static variables.

Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com>

dotnet/src/Connectors/Connectors.Memory.Pinecone/IPineconeClient.cs

@@ -21,7 +21,7 @@ public interface IPineconeClient
     /// <param name="includeValues"> Whether to include the vector values</param>
     /// <param name="cancellationToken"> The cancellation token</param>
     /// <returns> A list of vector records</returns>
-    public IAsyncEnumerable<PineconeDocument?> FetchVectorsAsync(
+    IAsyncEnumerable<PineconeDocument?> FetchVectorsAsync(
         string indexName,
         IEnumerable<string> ids,
         string indexNamespace = "",
@@ -38,7 +38,7 @@ public interface IPineconeClient
     /// <param name="includeMetadata"> whether to include the metadata</param>
     /// <param name="cancellationToken"></param>
     /// <returns> a list of query matches</returns>
-    public IAsyncEnumerable<PineconeDocument?> QueryAsync(
+    IAsyncEnumerable<PineconeDocument?> QueryAsync(
         string indexName,
         Query query,
         bool includeValues = false,
@@ -57,7 +57,7 @@ public interface IPineconeClient
     /// <param name="indexNamespace">The name assigned to a collection of vectors.</param>
     /// <param name="filter"> A filter to apply to the results</param>
     /// <param name="cancellationToken">Cancellation token.</param>
-    public IAsyncEnumerable<(PineconeDocument, double)> GetMostRelevantAsync(
+    IAsyncEnumerable<(PineconeDocument, double)> GetMostRelevantAsync(
         string indexName,
         IEnumerable<float> vector,
         double threshold,
@@ -160,7 +160,7 @@ Task UpdateAsync(
     /// </summary>
     /// <param name="indexName">The name assigned to a collection of vectors.</param>
     /// <param name="cancellationToken">Cancellation Token.</param>
-    public Task<bool> DoesIndexExistAsync(string indexName, CancellationToken cancellationToken = default);
+    Task<bool> DoesIndexExistAsync(string indexName, CancellationToken cancellationToken = default);
 
     /// <summary>
     /// Describe index

dotnet/src/Connectors/Connectors.Memory.Pinecone/PineconeClient.cs

@@ -12,7 +12,6 @@
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Abstractions;
-using Microsoft.SemanticKernel.Connectors.Memory.Pinecone.Http;
 using Microsoft.SemanticKernel.Connectors.Memory.Pinecone.Http.ApiSchema;
 using Microsoft.SemanticKernel.Connectors.Memory.Pinecone.Model;
 
@@ -21,21 +20,22 @@ namespace Microsoft.SemanticKernel.Connectors.Memory.Pinecone;
 /// <summary>
 /// A client for the Pinecone API
 /// </summary>
-public sealed class PineconeClient : IPineconeClient, IDisposable
+public sealed class PineconeClient : IPineconeClient
 {
     /// <summary>
     /// Initializes a new instance of the <see cref="PineconeClient"/> class.
     /// </summary>
-    /// <param name="pineconeEnvironment"></param>
-    /// <param name="apiKey"></param>
-    /// <param name="logger"></param>
-    public PineconeClient(string pineconeEnvironment, string apiKey, ILogger? logger = null)
+    /// <param name="pineconeEnvironment">The environment for Pinecone.</param>
+    /// <param name="apiKey">The API key for accessing Pinecone services.</param>
+    /// <param name="logger">An optional logger instance for logging.</param>
+    /// <param name="httpClient">An optional HttpClient instance for making HTTP requests.</param>
+    public PineconeClient(string pineconeEnvironment, string apiKey, ILogger? logger = null, HttpClient? httpClient = null)
     {
         this._pineconeEnvironment = pineconeEnvironment;
         this._authHeader = new KeyValuePair<string, string>("Api-Key", apiKey);
         this._jsonSerializerOptions = PineconeUtils.DefaultSerializerOptions;
         this._logger = logger ?? NullLogger<PineconeClient>.Instance;
-        this._httpClient = new HttpClient(HttpHandlers.CheckCertificateRevocation);
+        this._httpClient = httpClient ?? new HttpClient(NonDisposableHttpClientHandler.Instance, disposeHandler: false);
         this._indexHostMapping = new ConcurrentDictionary<string, string>();
     }
 
@@ -523,12 +523,6 @@ public async Task ConfigureIndexAsync(string indexName, int replicas = 1, PodTyp
         this._logger.LogDebug("Collection created. {0}", indexName);
     }
 
-    /// <inheritdoc />
-    public void Dispose()
-    {
-        this._httpClient.Dispose();
-    }
-
     #region private ================================================================================
 
     private readonly string _pineconeEnvironment;

dotnet/src/Connectors/Connectors.Memory.Pinecone/PineconeKernelBuilderExtensions.cs

@@ -0,0 +1,61 @@
+// Copyright (c) Microsoft. All rights reserved.
+
+using System.Net.Http;
+using Microsoft.Extensions.Logging;
+using Microsoft.SemanticKernel.Connectors.Memory.Pinecone;
+
+#pragma warning disable IDE0130
+namespace Microsoft.SemanticKernel;
+#pragma warning restore IDE0130
+
+/// <summary>
+/// Provides extension methods for the <see cref="KernelBuilder"/> class to configure Pinecone connectors.
+/// </summary>
+public static class PineconeKernelBuilderExtensions
+{
+    /// <summary>
+    /// Registers Pinecone Memory Store.
+    /// </summary>
+    /// <param name="builder">The <see cref="KernelBuilder"/> instance</param>
+    /// <param name="environment">The environment for Pinecone.</param>
+    /// <param name="apiKey">The API key for accessing Pinecone services.</param>
+    /// <param name="httpClient">An optional HttpClient instance for making HTTP requests.</param>
+    /// <returns>Self instance</returns>
+    public static KernelBuilder WithPineconeMemoryStore(this KernelBuilder builder,
+        string environment,
+        string apiKey,
+        HttpClient? httpClient = null)
+    {
+        builder.WithMemoryStorage((parameters) =>
+        {
+            var client = new PineconeClient(
+                environment,
+                apiKey,
+                parameters.Logger,
+                GetHttpClient(parameters.Config, httpClient, parameters.Logger));
+
+            return new PineconeMemoryStore(client, parameters.Logger);
+        });
+
+        return builder;
+    }
+
+    /// <summary>
+    /// Retrieves an instance of HttpClient.
+    /// </summary>
+    /// <param name="config">The kernel configuration.</param>
+    /// <param name="httpClient">An optional pre-existing instance of HttpClient.</param>
+    /// <param name="logger">An optional logger.</param>
+    /// <returns>An instance of HttpClient.</returns>
+    private static HttpClient GetHttpClient(KernelConfig config, HttpClient? httpClient, ILogger? logger)
+    {
+        if (httpClient == null)
+        {
+            var retryHandler = config.HttpHandlerFactory.Create(logger);
+            retryHandler.InnerHandler = NonDisposableHttpClientHandler.Instance;
+            return new HttpClient(retryHandler, false); // We should refrain from disposing the underlying SK default HttpClient handler as it would impact other HTTP clients that utilize the same handler.
+        }
+
+        return httpClient;
+    }
+}

dotnet/src/Connectors/Connectors.UnitTests/Memory/Pinecone/PineconeKernelBuilderExtensionsTests.cs

@@ -0,0 +1,54 @@
+// Copyright (c) Microsoft. All rights reserved.
+
+using System;
+using System.Linq;
+using System.Net.Http;
+using System.Net.Mime;
+using System.Text;
+using System.Threading.Tasks;
+using Microsoft.SemanticKernel;
+using Xunit;
+
+namespace SemanticKernel.Connectors.UnitTests.Memory.Pinecone;
+
+public sealed class PineconeKernelBuilderExtensionsTests : IDisposable
+{
+    private HttpMessageHandlerStub messageHandlerStub;
+    private HttpClient httpClient;
+
+    public PineconeKernelBuilderExtensionsTests()
+    {
+        this.messageHandlerStub = new HttpMessageHandlerStub();
+
+        this.httpClient = new HttpClient(this.messageHandlerStub, false);
+    }
+
+    [Fact]
+    public async Task PineconeMemoryStoreShouldBeProperlyInitialized()
+    {
+        //Arrange
+        this.messageHandlerStub.ResponseToReturn.Content = new StringContent("[\"fake-index1\"]", Encoding.UTF8, MediaTypeNames.Application.Json);
+
+        var builder = new KernelBuilder();
+        builder.WithPineconeMemoryStore("fake-environment", "fake-api-key", this.httpClient);
+        builder.WithAzureTextEmbeddingGenerationService("fake-deployment-name", "https://fake-random-test-host/fake-path", "fake -api-key");
+        var kernel = builder.Build(); //This call triggers the internal factory registered by WithPineconeMemoryStore method to create an instance of the PineconeMemoryStore class.
+
+        //Act
+        await kernel.Memory.GetCollectionsAsync(); //This call triggers a subsequent call to Pinecone memory store.
+
+        //Assert
+        Assert.Equal("https://controller.fake-environment.pinecone.io/databases", this.messageHandlerStub?.RequestUri?.AbsoluteUri);
+
+        var headerValues = Enumerable.Empty<string>();
+        var headerExists = this.messageHandlerStub?.RequestHeaders?.TryGetValues("Api-Key", out headerValues);
+        Assert.True(headerExists);
+        Assert.Contains(headerValues!, (value) => value == "fake-api-key");
+    }
+
+    public void Dispose()
+    {
+        this.httpClient.Dispose();
+        this.messageHandlerStub.Dispose();
+    }
+}

dotnet/src/SemanticKernel/KernelBuilder.cs

@@ -23,7 +23,7 @@ public sealed class KernelBuilder
     private KernelConfig _config = new();
     private ISemanticTextMemory _memory = NullMemory.Instance;
     private ILogger _logger = NullLogger.Instance;
-    private IMemoryStore? _memoryStorage = null;
+    private Func<IMemoryStore>? _memoryStorageFactory = null;
     private IDelegatingHandlerFactory? _httpHandlerFactory = null;
     private IPromptTemplateEngine? _promptTemplateEngine;
     private readonly AIServiceCollection _aiServices = new();
@@ -61,9 +61,9 @@ public IKernel Build()
         );
 
         // TODO: decouple this from 'UseMemory' kernel extension
-        if (this._memoryStorage != null)
+        if (this._memoryStorageFactory != null)
         {
-            instance.UseMemory(this._memoryStorage);
+            instance.UseMemory(this._memoryStorageFactory.Invoke());
         }
 
         return instance;
@@ -101,7 +101,19 @@ public KernelBuilder WithMemory(ISemanticTextMemory memory)
     public KernelBuilder WithMemoryStorage(IMemoryStore storage)
     {
         Verify.NotNull(storage);
-        this._memoryStorage = storage;
+        this._memoryStorageFactory = () => storage;
+        return this;
+    }
+
+    /// <summary>
+    /// Add memory storage factory to the kernel.
+    /// </summary>
+    /// <param name="factory">The storage factory.</param>
+    /// <returns>Updated kernel builder including the memory storage.</returns>
+    public KernelBuilder WithMemoryStorage<TStore>(Func<(ILogger Logger, KernelConfig Config), TStore> factory) where TStore : IMemoryStore
+    {
+        Verify.NotNull(factory);
+        this._memoryStorageFactory = () => factory((this._logger, this._config));
         return this;
     }
 

samples/dotnet/kernel-syntax-examples/Example38_Pinecone.cs

@@ -32,6 +32,7 @@ public static async Task RunAsync()
             .WithOpenAITextCompletionService("text-davinci-003", Env.Var("OPENAI_API_KEY"))
             .WithOpenAITextEmbeddingGenerationService("text-embedding-ada-002", Env.Var("OPENAI_API_KEY"))
             .WithMemoryStorage(memoryStore)
+            //.WithPineconeMemoryStore(pineconeEnvironment, apiKey) // This method offers an alternative approach to registering Pinecone memory storage.
             .Build();
 
         Console.WriteLine("== Printing Collections in DB ==");