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

community[patch]: fix astra initialize error #4502

Merged
merged 3 commits into from
Feb 24, 2024

Conversation

nicoloboschi
Copy link
Contributor

Currently in the AstraDBVectorStore#initialize method if there is an error while creating the collection, the error is suppressed, assuming the collection already exists. This is wrong because:

  • if the collection already exists with the same options, the Astra Data API won't return errors
  • In case of auth, network or server errors, the user can't really see the errors and no error is raised

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Feb 23, 2024
Copy link

vercel bot commented Feb 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchainjs-api-refs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 24, 2024 4:28pm
langchainjs-docs ✅ Ready (Inspect) Visit Preview Feb 24, 2024 4:28pm

@dosubot dosubot bot added the auto:bug Related to a bug, vulnerability, unexpected error with an existing feature label Feb 23, 2024
@jacoblee93
Copy link
Collaborator

Thank you!

@Gr33nLight
Copy link
Contributor

Hello regarding this, I noticed a pretty big impact of the call this.astraDBClient.createCollection in the initialize in terms of response time. Even if the collection already exists, this call takes multiple seconds to complete (2.2 to 2.5s im my testing). This is not ideal if the collection is already present, may a suggest in a pull request the option to disable automatic collection creation?

@nicoloboschi @jacoblee93

@nicoloboschi
Copy link
Contributor Author

@Gr33nLight I don't think it's related to the pull itself. the API calls are the same.
Checking if the collection already exists would take the same time.
Skipping the collection creation is very risky from a usability point of view 🤔

@Gr33nLight
Copy link
Contributor

Yes I agree in regards to the usability, It is more of a specific case, in our product speed is a priority and having 2s added to every request is not the best. We are using a modified version so its not that big of a issue for us, but I wanted to point this out anyways as I was not experiencing this slow down with the other adapters.
Maybe I'll check with the guys at Datastax to see if something can be done at client level, to maybe speed up some calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:bug Related to a bug, vulnerability, unexpected error with an existing feature size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants