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: Add support for Upstash Vector #20824

Merged
merged 56 commits into from
Apr 29, 2024
Merged

Conversation

CahidArda
Copy link
Contributor

Description

Adding UpstashVectorStore to utilize Upstash Vector!

#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 was added and we add this to our vectorstore aswell.

Dependencies

upstash-vector should be installed to use UpstashVectorStore. Didn't update dependencies because of this comment in the previous PR.

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. We didn't make changes to this end yet. We can update the tests if you can explain how the tests should be mocked.

auto-merge was automatically disabled April 25, 2024 19:03

Head branch was pushed to by a user without write access

@CahidArda
Copy link
Contributor Author

Thanks for the review @baskaryan!

Should we resolve all the failing CI checks to merge this PR or are we waiting for review/approval from other people?

I didn't try to resolve all CI issues because we want to offer some functionality on top of what is possible with the VectorStore interface (like embeddings). Since you approved the PR I am not making further changes

@baskaryan
Copy link
Collaborator

Thanks for the review @baskaryan!

Should we resolve all the failing CI checks to merge this PR or are we waiting for review/approval from other people?

I didn't try to resolve all CI issues because we want to offer some functionality on top of what is possible with the VectorStore interface (like embeddings). Since you approved the PR I am not making further changes

Resolving CI failures would be great!

I had included nest_asyncio.apply in the test to stop an error I was getting: "RuntimeError: asyncio.run() cannot be called from a running event loop"

But nest_asyncio couldn't be imported in CI so I had to remove it. To fix the error, I had to downgrade vcrpy to 4.3.0 and urllib3 to 1.26.18 according to poetry.lock. This solved the error I was getting.

This also meant that the cassettes changed. Updating them aswell.
@CahidArda
Copy link
Contributor Author

Hi again @baskaryan,

CI errors are resolved. Used VCR to record the integration tests aswell!

@baskaryan baskaryan enabled auto-merge (squash) April 29, 2024 15:41
@CahidArda
Copy link
Contributor Author

Hi @baskaryan,

It looks like all tests pass except Vercel deployment, which got Canceled by a more recent commit in the same branch error

@baskaryan baskaryan disabled auto-merge April 29, 2024 21:24
@baskaryan baskaryan enabled auto-merge (squash) April 29, 2024 21:24
@baskaryan baskaryan disabled auto-merge April 29, 2024 21:24
@baskaryan baskaryan merged commit cc6191c into langchain-ai:master Apr 29, 2024
60 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features lgtm PR looks good. Use to confirm that a PR is ready for merging. size:XXL This PR changes 1000+ lines, ignoring generated files. Ɑ: vector store Related to vector store module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants