Skip to content
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

Validate index name parameter #29

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

B-Step62
Copy link
Collaborator

Previously in LC community version, DatabricksVectorSearch had index as the first argument of the constructor, which only accepts the index Python object returned from VS SDK's get_index API. This is inconvenient and also troublesome for migrating to Databricks SDK, so we replaced it with the index_name string argument in the new package.

Some customers are not aware of this and still pass the index object. This will lead to an error from VS client saying Invalid index name. Must specify the... which is not clear because the index itself has correct name.

This PR adds a validation for the argument to show clearer error message.

@B-Step62 B-Step62 force-pushed the add-type-validation branch from 2fb11c0 to ce3ac0f Compare October 18, 2024 00:23
@B-Step62 B-Step62 requested a review from BenWilson2 October 18, 2024 00:32
@B-Step62
Copy link
Collaborator Author

@BenWilson2 This is a follow-up for the ES ticket - the issue was in users' code but this PR makes debugging easier. Would appreciate if you could take a look, thanks!

Copy link
Collaborator

@BenWilson2 BenWilson2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great quality of life improvement for users! Thanks for adding this @B-Step62 !!

@B-Step62 B-Step62 merged commit 91e9810 into langchain-ai:main Oct 18, 2024
8 checks passed
@B-Step62 B-Step62 deleted the add-type-validation branch October 18, 2024 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants