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[minor]: VectorStore integration for SAP HANA Cloud Vector Engine #4984

Merged
merged 35 commits into from
Apr 12, 2024

Conversation

cinqisap
Copy link
Contributor

@cinqisap cinqisap commented Apr 5, 2024

Implementation of the integration:
libs/langchain-community/src/vectorstores/hanavector.ts

Unit tests:
libs/langchain-community/src/vectorstores/tests/hanavector.test.ts

Integration tests:
libs/langchain-community/src/vectorstores/tests/hanavector.int.test.ts

Examples:
examples/src/indexes/vector_stores/hana_vector/

Documentation:
docs/core_docs/docs/integrations/vectorstores/hanavector.mdx

Integration tests and examples were run successfully.
Access credentials for the execution of the integration tests can be provided to the maintainers.

Copy link

vercel bot commented Apr 5, 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 Apr 12, 2024 9:53pm
langchainjs-docs ✅ Ready (Inspect) Visit Preview Apr 12, 2024 9:53pm

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. auto:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features labels Apr 5, 2024
@jacoblee93
Copy link
Collaborator

Hi @cinqisap, thanks so much for this!

The SAP client dep is quite large (186MB!) and I'm worried it'll slow down our CI. I see that it's only being used as a type in the connector itself - is there any lighter variant?

@jacoblee93
Copy link
Collaborator

jacoblee93 commented Apr 5, 2024

Yeah, it adds a full minute to our yarn install. The PR itself looks fine though!

If there is no lighter version, what we can do is split it out into a partner package that is versioned separately, and skip building it in CI.

The other option I see is to inline the examples/ in the docs, comment out the test cases, and define some other interface for hanaClient.Connection or even type it as any so that the client isn't actually a direct dep.

What do you prefer?

@cinqisap
Copy link
Contributor Author

hi, @jacoblee93, thanks for the suggestion!

I pushed a new commit to address this issue. My solution is to add support for another HANA Node.js driver hdb which is only about 1MB and thus remove the large @sap/hana-client as a direct dev package. Since the hdb did not originally support typescript, I added another declaration file to avoid errors during building time.

Now the client type is defined as any and can represent both drivers.

Looking forward to hearing more if the current solution is not as desired.

@jacoblee93
Copy link
Collaborator

Yes that's a great solution! Thank you, I'll look shortly.

@@ -0,0 +1 @@
declare module "hdb";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fine to add this as a dep to examples/ - will do that shortly.

@jacoblee93
Copy link
Collaborator

Ah, sorry missed the part about the declaration. Will fix.

@jacoblee93 jacoblee93 added lgtm PRs that are ready to be merged as-is and removed question Further information is requested close PRs that need one or two touch-ups to be ready labels Apr 12, 2024
@jacoblee93 jacoblee93 changed the title community: VectorStore integration for SAP HANA Cloud Vector Engine community[minor]: VectorStore integration for SAP HANA Cloud Vector Engine Apr 12, 2024
@jacoblee93 jacoblee93 merged commit 0634402 into langchain-ai:main Apr 12, 2024
17 checks passed
@jacoblee93
Copy link
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features lgtm PRs that are ready to be merged as-is size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants