Remove TODO and test helper through caller

Signed-off-by: owenhalpert <ohalpert@gmail.com>
owenhalpert · Mar 4, 2025 · 3e5746e · 3e5746e
1 parent aef03d0
commit 3e5746e
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 9 deletions.
diff --git a/src/main/java/org/opensearch/knn/index/engine/faiss/FaissHNSWMethod.java b/src/main/java/org/opensearch/knn/index/engine/faiss/FaissHNSWMethod.java
@@ -153,7 +153,6 @@ protected Function<TrainingConfigValidationInput, TrainingConfigValidationOutput
         };
     }
 
-    // TODO refactor to make the index parameter fetching more intelligent and less cumbersome
     /**
      * Get the parameters that need to be passed to the remote build service for training
      *
@@ -180,13 +179,12 @@ public static RemoteIndexParameters getRemoteIndexingParameters(Map<String, Obje
         return builder.build();
     }
 
-    public static int getMFromIndexDescription(String indexDescription) {
+    private static int getMFromIndexDescription(String indexDescription) {
         int commaIndex = indexDescription.indexOf(",");
         if (commaIndex == -1) {
             throw new IllegalArgumentException("Invalid index description: " + indexDescription);
         }
         String hnswPart = indexDescription.substring(0, commaIndex);
-        int m = Integer.parseInt(hnswPart.substring(4));
-        return m;
+        return Integer.parseInt(hnswPart.substring(4));
     }
 }
diff --git a/src/test/java/org/opensearch/knn/index/remote/RemoteIndexHTTPClientTests.java b/src/test/java/org/opensearch/knn/index/remote/RemoteIndexHTTPClientTests.java
@@ -69,7 +69,7 @@
 import static org.opensearch.knn.index.KNNSettings.KNN_REMOTE_BUILD_SERVICE_ENDPOINT_SETTING;
 import static org.opensearch.knn.index.SpaceType.L2;
 import static org.opensearch.knn.index.VectorDataType.FLOAT;
-import static org.opensearch.knn.index.engine.faiss.FaissHNSWMethod.getMFromIndexDescription;
+import static org.opensearch.knn.index.engine.faiss.FaissHNSWMethod.getRemoteIndexingParameters;
 import static org.opensearch.knn.index.remote.KNNRemoteConstants.BUCKET;
 import static org.opensearch.knn.index.remote.KNNRemoteConstants.BUILD_ENDPOINT;
 import static org.opensearch.knn.index.remote.KNNRemoteConstants.DOC_ID_FILE_EXTENSION;
@@ -107,10 +107,19 @@ public void testGetAndCloseHttpclient_success() throws IOException {
         client.close();
     }
 
-    public void testGetMFromIndexDescription() {
-        assertEquals(16, getMFromIndexDescription("HNSW16,Flat"));
-        assertEquals(8, getMFromIndexDescription("HNSW8,SQ"));
-        assertThrows(IllegalArgumentException.class, () -> getMFromIndexDescription("Invalid description"));
+    public void testGetRemoteIndexingParameters_Success() {
+        BuildIndexParams params = createTestBuildIndexParams();
+        RemoteIndexParameters result = getRemoteIndexingParameters(params.getParameters());
+
+        assertNotNull(result);
+        assertTrue(result instanceof RemoteFaissHNSWIndexParameters);
+
+        RemoteFaissHNSWIndexParameters hnswParams = (RemoteFaissHNSWIndexParameters) result;
+        assertEquals(METHOD_HNSW, hnswParams.algorithm);
+        assertEquals("l2", hnswParams.spaceType);
+        assertEquals(94, hnswParams.efConstruction);
+        assertEquals(89, hnswParams.efSearch);
+        assertEquals(14, hnswParams.m);
     }
 
     /**