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
feat(langchain4j-milvus): MilvusEmbeddingStore supports configure required index parameters #931
base: main
Are you sure you want to change the base?
Conversation
…ent builder This commit addresses an issue where the baseUrl in the builder of ZhipuAiClient was not properly configured in Module langchain4j-zhipu-ai. This misconfiguration could potentially lead to confusion or errors when using the client directly. By correcting the baseUrl configuration, this commit ensures consistent and accurate behavior of the client.
…uired index parameters Fix: [langchain4j#860](langchain4j#860) Added support for configure common custom index parameters BREAKING CHANGE: The constructor of MilvusEmbeddingStore now require a parameter of type IndexParam.
@langchain4j Is there a configured milvus instance available for CI unit testing? Attempting to connect using MILVUS_URI and MILVUS_API_KEY retrieved from environment variables seems to fail. |
You can use the @EnabledIfEnvironmentVariable(named = "MILVUS_API_KEY", matches = ".+")
class MilvusEmbeddingStoreCloudIT extends EmbeddingStoreWithFilteringIT {
// Test class content
} Additionally, I suggest that |
This PR mainly aims to add parameters for Index creation. Collection and index creation are performed within the constructor of Additionally, regarding modifications to other methods in |
The Milvus instance in unit testing has been adjusted to be provided by MilvusContainer milvus = new MilvusContainer("milvusdb/milvus:v2.3.1")
.withCreateContainerCmdModifier(cmd -> {
cmd.withHostConfig(HostConfig.newHostConfig()
// This security opts needs to be configured;
// otherwise, the container will fail to start.
// Refer to the parameters from https://raw.githubusercontent.com/milvus-io/milvus/master/scripts/standalone_embed.sh.
.withSecurityOpts(Collections.singletonList("seccomp=unconfined"))
// By default, the Milvus Docker image doesn't expose the target port.
// Manual binding is required to expose it;
// otherwise, it will result in a failed detection during the test container inspection,
// leading to an inability to connect to the Milvus instance.
.withPortBindings(PortBinding.parse("19530:19530"), PortBinding.parse("9091:9091")
)
);
}); |
No, not for the main CI yet. |
Context
Added support for configure common custom index parameters
Fix: #860
Change
The constructor of MilvusEmbeddingStore now require a parameter of type IndexParam and provides parameter models required for common type indexes.
Checklist
Before submitting this PR, please check the following points:
Checklist for adding new embedding store integration