Address PR comments

momentohq · Oct 30, 2023 · 8002a43 · 8002a43
1 parent f86ce06
commit 8002a43
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 60 deletions.
diff --git a/src/Momento.Sdk/Config/Transport/StaticVectorIndexTransportStrategy.cs b/src/Momento.Sdk/Config/Transport/StaticVectorIndexTransportStrategy.cs
@@ -49,7 +49,7 @@ public override bool Equals(object obj)
             return false;
         }
 
-        var other = (StaticTransportStrategy)obj;
+        var other = (StaticVectorIndexTransportStrategy)obj;
         return GrpcConfig.Equals(other.GrpcConfig);
     }
 

diff --git a/src/Momento.Sdk/Config/VectorIndexConfigurations.cs b/src/Momento.Sdk/Config/VectorIndexConfigurations.cs
@@ -55,55 +55,4 @@ public static IVectorIndexConfiguration V1(ILoggerFactory? loggerFactory = null)
             return new Laptop(finalLoggerFactory, transportStrategy);
         }
     }
-
-    /// <summary>
-    /// InRegion provides defaults suitable for an environment where your client is running in the same region as the Momento
-    /// service.  It has more agressive timeouts and retry behavior than the Laptop config.
-    /// </summary>
-    public class InRegion
-    {
-        /// <summary>
-        ///  This config prioritizes throughput and client resource utilization.
-        /// </summary>
-        public class Default : VectorIndexConfiguration
-        {
-            private Default(ILoggerFactory loggerFactory, IVectorIndexTransportStrategy transportStrategy)
-                : base(loggerFactory, transportStrategy)
-            {
-
-            }
-
-            /// <summary>
-            /// Provides the latest recommended configuration for an InRegion environment.
-            /// </summary>
-            /// <remark>
-            /// This configuration may change in future releases to take advantage of
-            /// improvements we identify for default configurations.
-            /// </remark>
-            /// <param name="loggerFactory"></param>
-            /// <returns></returns>
-            public static IVectorIndexConfiguration Latest(ILoggerFactory? loggerFactory = null)
-            {
-                return V1(loggerFactory);
-            }
-
-            /// <summary>
-            /// Provides the version 1 configuration for an InRegion environment.
-            /// </summary>
-            /// <remark>
-            /// This configuration is guaranteed not to change in future
-            /// releases of the Momento .NET SDK.
-            /// </remark>
-            /// <param name="loggerFactory"></param>
-            /// <returns></returns>
-            public static IVectorIndexConfiguration V1(ILoggerFactory? loggerFactory = null)
-            {
-                var finalLoggerFactory = loggerFactory ?? NullLoggerFactory.Instance;
-                IVectorIndexTransportStrategy transportStrategy = new StaticVectorIndexTransportStrategy(
-                    loggerFactory: finalLoggerFactory,
-                    grpcConfig: new StaticGrpcConfiguration(deadline: TimeSpan.FromMilliseconds(1100)));
-                return new Default(finalLoggerFactory, transportStrategy);
-            }
-        }
-    }
 }
diff --git a/src/Momento.Sdk/IPreviewVectorIndexClient.cs b/src/Momento.Sdk/IPreviewVectorIndexClient.cs
@@ -33,7 +33,7 @@ public interface IPreviewVectorIndexClient: IDisposable
     /// Pattern matching can be used to operate on the appropriate subtype.
     /// For example:
     /// <code>
-    /// if (response is CreateCacheResponse.Error errorResponse)
+    /// if (response is CreateVectorIndexResponse.Error errorResponse)
     /// {
     ///     // handle error as appropriate
     /// }
@@ -57,7 +57,7 @@ public interface IPreviewVectorIndexClient: IDisposable
     /// </list>
     /// </remarks>
     /// </returns>
-    public Task<CreateVectorIndexResponse> CreateIndexAsync(string indexName, ulong numDimensions, SimilarityMetric similarityMetric = SimilarityMetric.CosineSimilarity);
+    public Task<CreateVectorIndexResponse> CreateIndexAsync(string indexName, long numDimensions, SimilarityMetric similarityMetric = SimilarityMetric.CosineSimilarity);
 
     /// <summary>
     /// Lists all vector indexes.

diff --git a/src/Momento.Sdk/Internal/VectorIndexControlClient.cs b/src/Momento.Sdk/Internal/VectorIndexControlClient.cs
@@ -25,14 +25,14 @@ public VectorIndexControlClient(ILoggerFactory loggerFactory, string authToken,
         _exceptionMapper = new CacheExceptionMapper(loggerFactory);
     }
 
-    public async Task<CreateVectorIndexResponse> CreateIndexAsync(string indexName, ulong numDimensions, SimilarityMetric similarityMetric)
+    public async Task<CreateVectorIndexResponse> CreateIndexAsync(string indexName, long numDimensions, SimilarityMetric similarityMetric)
     {
         try
         {
             _logger.LogTraceVectorIndexRequest("createVectorIndex", indexName);
             CheckValidIndexName(indexName);
-            CheckValidNumDimensions(numDimensions);
-            var request = new _CreateIndexRequest { IndexName = indexName, NumDimensions = numDimensions };
+            var validatedNumDimensions = ValidateNumDimensions(numDimensions);
+            var request = new _CreateIndexRequest { IndexName = indexName, NumDimensions = validatedNumDimensions };
             switch (similarityMetric)
             {
                 case SimilarityMetric.CosineSimilarity:
@@ -101,12 +101,14 @@ private static void CheckValidIndexName(string indexName)
         }
     }
 
-    private static void CheckValidNumDimensions(ulong numDimensions)
+    private static ulong ValidateNumDimensions(long numDimensions)
     {
         if (numDimensions <= 0)
         {
             throw new InvalidArgumentException("numDimensions must be greater than 0");
         }
+
+        return (ulong)numDimensions;
     }
 
     private DateTime CalculateDeadline()

diff --git a/src/Momento.Sdk/PreviewVectorIndexClient.cs b/src/Momento.Sdk/PreviewVectorIndexClient.cs
@@ -7,6 +7,12 @@
 
 namespace Momento.Sdk;
 
+/// <summary>
+/// PREVIEW Vector Index Client implementation
+/// WARNING: the API for this client is not yet stable and may change without notice.
+///
+/// Includes control operations and data operations.
+/// </summary>
 public class PreviewVectorIndexClient: IPreviewVectorIndexClient
 {
     private readonly VectorIndexControlClient controlClient;
@@ -27,7 +33,7 @@ public PreviewVectorIndexClient(IVectorIndexConfiguration config, ICredentialPro
     }
 
     /// <inheritdoc />
-    public async Task<CreateVectorIndexResponse> CreateIndexAsync(string indexName, ulong numDimensions,
+    public async Task<CreateVectorIndexResponse> CreateIndexAsync(string indexName, long numDimensions,
         SimilarityMetric similarityMetric = SimilarityMetric.CosineSimilarity)
     {
         return await controlClient.CreateIndexAsync(indexName, numDimensions, similarityMetric);

diff --git a/tests/Integration/Momento.Sdk.Tests/Fixtures.cs b/tests/Integration/Momento.Sdk.Tests/Fixtures.cs
@@ -135,7 +135,7 @@ public class VectorIndexClientFixture : IDisposable
     public VectorIndexClientFixture()
     {
         AuthProvider = new EnvMomentoTokenProvider("TEST_AUTH_TOKEN");
-        Client = new PreviewVectorIndexClient(VectorIndexConfigurations.InRegion.Default.Latest(LoggerFactory.Create(builder =>
+        Client = new PreviewVectorIndexClient(VectorIndexConfigurations.Laptop.Latest(LoggerFactory.Create(builder =>
         {
             builder.AddSimpleConsole(options =>
             {