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.

Sign up for GitHub

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

Jump to bottom

community: Add support for Upstash Vector #17012

Closed
wants to merge 38 commits into from
Closed

community: Add support for Upstash Vector #17012

wants to merge 38 commits into from

Conversation

ytkimirti
Copy link
Contributor

This PR adds support for Upstash Vector database.

@vercel Vercel
Copy link

vercel bot commented Feb 4, 2024 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 20, 2024 10:50am

@baskaryan
Copy link
Collaborator

@ytkimirti feel free to tag when pr is ready for review!

@ytkimirti
Copy link
Contributor Author

Hey @baskaryan, the code and integration tests are ready for review. If everything is ok I will start filling up the docs.

One question about dependencies, should I add upstash-vector as an optional dependency to /libs/community/pyproject.toml?
I was not sure because I couldn't find pinecone or chromadb there

@ytkimirti
Copy link
Contributor Author

@baskaryan bump

baskaryan
baskaryan reviewed Feb 12, 2024
View reviewed changes
Copy link
Collaborator

@baskaryan baskaryan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

code looks good to me! no need to add as optional dependency

if helpful, you can run local formatting and linting with:

cd libs/community
make format
make lint

libs/community/langchain_community/vectorstores/upstash.py Outdated Show resolved Hide resolved
libs/community/langchain_community/vectorstores/upstash.py Show resolved Hide resolved
@ytkimirti
Copy link
Contributor Author

Thanks for the review, I will complete the docs and examples in a few days

About the new integration tests, they need these environment variables, you can create them from here if needed

UPSTASH_VECTOR_URL=your_upstash_vector_url
UPSTASH_VECTOR_TOKEN=your_upstash_vector_token

@ytkimirti ytkimirti marked this pull request as ready for review February 18, 2024 17:49
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. Ɑ: vector store Related to vector store module 🤖:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features labels Feb 18, 2024
@ytkimirti
Copy link
Contributor Author

Hello, @baskaryan! The pr is ready for merging. I've ran the integration tests locally and also tested the retriever API—everything seems to work.

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Feb 19, 2024
efriis
efriis reviewed Feb 21, 2024
View reviewed changes
Copy link
Member

@efriis efriis left a comment

Choose a reason for hiding this comment

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

Hey there! These integration tests will not work as-is in community. Can we:

  • convert the integration tests into mocked unit tests

https://python.langchain.com/docs/contributing/testing#integration-tests

@efriis efriis removed the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Feb 21, 2024
@ytkimirti
Copy link
Contributor Author

Hey @efriis, do you mean mocking the upstash_vector library or just recording the requests using pytest-vcr? When writing the unit tests I looked at how other vectorstores wrote it (for example chroma) and I didn't see any mocking. Can you give more information about this?

@ytkimirti
Copy link
Contributor Author

bump @efriis

@ytkimirti
Copy link
Contributor Author

@baskaryan @efriis If it's not possible, we can remove the integration tests for now so we can merge this pr. What do you think about that?

@ytkimirti ytkimirti marked this pull request as draft March 7, 2024 07:46
@ytkimirti
Copy link
Contributor Author

Update: We are currently adding support for metadata filtering. I will update this pr when the feature is shipped to the sdk

@efriis
Copy link
Member

efriis commented Apr 8, 2024

closing for now and can reopen when that's in!

@efriis efriis closed this Apr 8, 2024
@CahidArda CahidArda mentioned this pull request Apr 24, 2024
Merged
baskaryan added a commit that referenced this pull request Apr 29, 2024
@CahidArda @ytkimirti @baskaryan
community[minor]: Add support for Upstash Vector (#20824)
cc6191c 
## Description

Adding `UpstashVectorStore` to utilize [Upstash
Vector](https://upstash.com/docs/vector/overall/getstarted)!

#17012 was opened to add Upstash Vector to langchain but was closed to
wait for filtering. Now filtering is added to Upstash vector and we open
a new PR. Additionally, [embedding
feature](https://upstash.com/docs/vector/features/embeddingmodels) was
added and we add this to our vectorstore aswell.

## Dependencies

[upstash-vector](https://pypi.org/project/upstash-vector/) should be
installed to use `UpstashVectorStore`. Didn't update dependencies
because of [this comment in the previous
PR](#17012 (review)).

## Tests

Tests are added and they pass. Tests are naturally network bound since
Upstash Vector is offered through an API.

There was [a discussion in the previous PR about mocking the
unittests](#17012 (review)).
We didn't make changes to this end yet. We can update the tests if you
can explain how the tests should be mocked.

---------

Co-authored-by: ytkimirti <[email protected]>
Co-authored-by: Bagatur <[email protected]>
Co-authored-by: Bagatur <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efriis efriis efriis left review comments

@baskaryan baskaryan baskaryan approved these changes

Assignees

@efriis efriis

Labels
🤖:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features size:XXL This PR changes 1000+ lines, ignoring generated files. Ɑ: vector store Related to vector store module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ytkimirti @baskaryan @efriis