Skip to content

Initial add IBM DB2 vector store . #68

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

Merged
merged 21 commits into from
Jun 20, 2025
Merged

Conversation

yiweiHeOSS
Copy link
Collaborator

@yiweiHeOSS yiweiHeOSS commented Mar 28, 2025

This PR adds a new folder under libs for the DB2 vector store langchain connector code, i.e., libs/langchain-db2.
Other companies are doing the same things for their vector store (e.g. Google, AWS and Azure). Thanks for @ccurme to give me some guidance about where to put the code and @MateuszOssGit gave me the access to this repo .

Later, I will create a package on Pypi of the vector store connector, i.e. pip install langchain-db2. Note that it should have no impact on the current package pip install langchain-ibm. I just need a separate directory libs/langchain-db2 to put open-source code.

The major changes in this PR are in the files below:

  • libs/langchain-db2/langchain_db2/db2vs.py - this is the source file of the db2 vector store
  • libs/langchain-db2/docs/db2.ipynb - this is the Jupyter notebook for the demo and steps on how to use the db2 vector store
  • libs/langchain-db2/tests/integration_tests/test_db2vs.py - This is the integration test

@yiweiHeOSS yiweiHeOSS self-assigned this Mar 28, 2025
@yiweiHeOSS yiweiHeOSS requested a review from shaikhq May 29, 2025 22:12
@yiweiHeOSS yiweiHeOSS requested a review from shaikhq June 5, 2025 02:32
Copy link
Collaborator

@shaikhq shaikhq left a comment

Choose a reason for hiding this comment

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

All changes look good to me.

@yiweiHeOSS
Copy link
Collaborator Author

Hi @MateuszOssGit @Mateusz-Switala @ccurme . Could you please review this PR? @shaikhq From our IBM DB2 team has approved it. It should be ready for you to review. If I need to do anything, please let me know. Thank you!

ccurme added 3 commits June 5, 2025 09:05
8 files left unchanged
[ "." = "" ] || poetry run ruff check --select I --fix .
All checks passed!)
Copy link
Contributor

@ccurme ccurme left a comment

Choose a reason for hiding this comment

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

It looks like CI was not enabled for the new library. I enabled it and added a placeholder test.

IMO, priorities are (in descending order):

  • Get CI checks to pass (this will block merging)
  • Get make integration_test to pass on CI (this will block release)
  • Incorporate the standard test suite for vector stores (this is optional but nice to have)

@yiweiHeOSS
Copy link
Collaborator Author

@ccurme Thanks for your comments. I tried to fix most of the problems in the CI, but I still saw lint problems. Do we have a command to fix them automatically? Do you see other issues I need to fix? Thanks.

@ccurme
Copy link
Contributor

ccurme commented Jun 6, 2025

@ccurme Thanks for your comments. I tried to fix most of the problems in the CI, but I still saw lint problems. Do we have a command to fix them automatically? Do you see other issues I need to fix? Thanks.

make format runs an auto-formatter but I don't think it will address the issues:

Error: langchain_db2/db2vs.py:296:89: E501 Line too long (99 > 88)
Error: langchain_db2/db2vs.py:334:89: E501 Line too long (158 > 88)
Error: langchain_db2/db2vs.py:455:9: F842 Local variable embedding_arr is annotated but never used

@MateuszOssGit
Copy link
Collaborator

I was able to successfully format all required changes to linting successfully passed.

@yiweiHeOSS @shaikhq Could you please check on my changes from PR above to verify if everything has been updated correctly with your requirements?

@yiweiHeOSS
Copy link
Collaborator Author

yiweiHeOSS commented Jun 6, 2025

@ccurme @MateuszOssGit Thank you so much for your help! It looks like this PR is good to review again.

@MateuszOssGit, I only found one little thing that I changed. It looks like we can't print() in the testcase (not sure if we have a better way to print the result in test), but we still want to run the functions like _embed_documents() for testing purposes.

@yiweiHeOSS yiweiHeOSS requested a review from ccurme June 6, 2025 20:36
@yiweiHeOSS yiweiHeOSS requested a review from MateuszOssGit June 9, 2025 21:50
@yiweiHeOSS
Copy link
Collaborator Author

Hi @ccurme @MateuszOssGit, I resolved all the comments. If everything is good, can you please approve the PR so we can deliver it? Thanks. cc @Mateusz-Switala

@yiweiHeOSS
Copy link
Collaborator Author

@ccurme @MateuszOssGit :)

@yiweiHeOSS
Copy link
Collaborator Author

Thank you for reviewing it @Mateusz-Switala @MateuszOssGit

@yiweiHeOSS
Copy link
Collaborator Author

Hi @ccurme, could you approve this PR? Thanks.

@yiweiHeOSS yiweiHeOSS merged commit 4933407 into langchain-ai:main Jun 20, 2025
22 checks passed
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.

5 participants