community[minor]: Add SQL storage implementation #22207

pprados · 2024-05-27T12:31:47Z

package: langchain-comminity
Description: Add SQL implementation for docstore. A new implementation, in line with my other PR (async PGVector, SQLChatMessageMemory)
Twitter handler: pprados

vercel · 2024-05-27T12:31:52Z

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	Jun 7, 2024 9:17pm

libs/community/langchain_community/storage/sql.py

eyurtsev

Basically ready to merge, we just need to resolve a few issues with the sql alchemy schema

…ins/agents/docs (langchain-ai#16168) Revamp SQL use cases docs. In the process update SQL chains and agents.

…corator (langchain-ai#16295) Adjusted `deprecate` decorator to make sure decorated async functions are still recognized as "coroutinefunction" by `inspect`. Before change, functions such as `LLMChain.acall` which are decorated as deprecated are not recognized as coroutine functions. After the change, they are recognized: ```python import inspect from langchain import LLMChain # Is false before change but true after. inspect.iscoroutinefunction(LLMChain.acall) ```

- **Description:** add milvus multitenancy doc, it is an example for this [pr](langchain-ai#15740) . - **Issue:** No, - **Dependencies:** No, - **Twitter handle:** No Signed-off-by: ChengZi <[email protected]>

pprados · 2024-06-07T11:42:19Z

@eyurtsev
It's correct now, but Merging is blocked because "@eyurtsev requested changes".

Please, can you check this other Add async mode for pgvector ? it has been blocked for 3 weeks because "@eyurtsev requested changes", but I can't find the changes I want, so I can't validate them

eyurtsev · 2024-06-07T20:58:18Z

libs/community/pyproject.toml

@@ -31,6 +31,7 @@ optional = true
 pytest = "^7.3.0"
 pytest-cov = "^4.1.0"
 pytest-dotenv = "^0.5.2"
+aiosqlite = "^0.19.0"


FYI @pprados aiosqlite should be in extended testing only not in this test group -- I'll move it to the appropriate place

this is reserved for testing infrastructure (pytest, pytest-cov, etc.)

There are a few things that don't belong here (e.g., duckdb-engine, lark), but we'll remove those as well at some point

OK it's already in extended testing, so we should be all set 🤞

eyurtsev · 2024-06-07T21:05:22Z

libs/community/tests/integration_tests/storage/test_sql.py

@@ -0,0 +1,186 @@
+"""Implement integration tests for Redis storage."""


I'd recommend removing integration tests since they duplicate the content of unit tests.

unit tests will run on CI right now under the extended tests run.

At the moment, integration tests are not run as part of CI (since they typically require access to external resources that are not set up)

@eyurtsev

…elf query retriever (#22678) The fact that we outsourced pgvector to another project has an unintended effect. The mapping dictionary found by `_get_builtin_translator()` cannot recognize the new version of pgvector because it comes from another package. `SelfQueryRetriever` no longer knows `PGVector`. I propose to fix this by creating a global dictionary that can be populated by various database implementations. Thus, importing `langchain_postgres` will allow the registration of the `PGvector` mapping. But for the moment I'm just adding a lazy import Furthermore, the implementation of _get_builtin_translator() reconstructs the BUILTIN_TRANSLATORS variable with each invocation, which is not very efficient. A global map would be an optimization. - **Twitter handle:** pprados @eyurtsev, can you review this PR? And unlock the PR [Add async mode for pgvector](langchain-ai/langchain-postgres#32) and PR [community[minor]: Add SQL storage implementation](#22207)? Are you in favour of a global dictionary-based implementation of Translator?

@eyurtsev

…elf query retriever (langchain-ai#22678) The fact that we outsourced pgvector to another project has an unintended effect. The mapping dictionary found by `_get_builtin_translator()` cannot recognize the new version of pgvector because it comes from another package. `SelfQueryRetriever` no longer knows `PGVector`. I propose to fix this by creating a global dictionary that can be populated by various database implementations. Thus, importing `langchain_postgres` will allow the registration of the `PGvector` mapping. But for the moment I'm just adding a lazy import Furthermore, the implementation of _get_builtin_translator() reconstructs the BUILTIN_TRANSLATORS variable with each invocation, which is not very efficient. A global map would be an optimization. - **Twitter handle:** pprados @eyurtsev, can you review this PR? And unlock the PR [Add async mode for pgvector](langchain-ai/langchain-postgres#32) and PR [community[minor]: Add SQL storage implementation](langchain-ai#22207)? Are you in favour of a global dictionary-based implementation of Translator?

@eyurtsev

Hello @eyurtsev - package: langchain-comminity - **Description**: Add SQL implementation for docstore. A new implementation, in line with my other PR ([async PGVector](langchain-ai/langchain-postgres#32), [SQLChatMessageMemory](#22065)) - Twitter handler: pprados --------- Signed-off-by: ChengZi <[email protected]> Co-authored-by: Bagatur <[email protected]> Co-authored-by: Piotr Mardziel <[email protected]> Co-authored-by: ChengZi <[email protected]> Co-authored-by: Eugene Yurtsev <[email protected]>

@eyurtsev

…elf query retriever (#22678) The fact that we outsourced pgvector to another project has an unintended effect. The mapping dictionary found by `_get_builtin_translator()` cannot recognize the new version of pgvector because it comes from another package. `SelfQueryRetriever` no longer knows `PGVector`. I propose to fix this by creating a global dictionary that can be populated by various database implementations. Thus, importing `langchain_postgres` will allow the registration of the `PGvector` mapping. But for the moment I'm just adding a lazy import Furthermore, the implementation of _get_builtin_translator() reconstructs the BUILTIN_TRANSLATORS variable with each invocation, which is not very efficient. A global map would be an optimization. - **Twitter handle:** pprados @eyurtsev, can you review this PR? And unlock the PR [Add async mode for pgvector](langchain-ai/langchain-postgres#32) and PR [community[minor]: Add SQL storage implementation](#22207)? Are you in favour of a global dictionary-based implementation of Translator?

xfailing some sql tests that do not currently work on sqlalchemy v1 #22207 was very much not sqlalchemy v1 compatible. Moving forward, implementations should be compatible with both to pass CI

xfailing some sql tests that do not currently work on sqlalchemy v1 langchain-ai#22207 was very much not sqlalchemy v1 compatible. Moving forward, implementations should be compatible with both to pass CI

vercel bot had a problem deploying to Preview May 27, 2024 12:37 Failure

pprados changed the title ~~Pprados/new sql docstore~~ Add SQL docstore May 27, 2024

vercel bot deployed to Preview May 27, 2024 12:58 View deployment

pprados force-pushed the pprados/new_sql_docstore branch 4 times, most recently from cbc83bb to 2671786 Compare May 27, 2024 13:16

vercel bot deployed to Preview May 27, 2024 13:31 View deployment

pprados marked this pull request as ready for review May 27, 2024 13:32

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. 🤖:improvement Medium size change to existing code to handle new use-cases labels May 27, 2024

baskaryan assigned eyurtsev May 27, 2024

eyurtsev approved these changes Jun 5, 2024

View reviewed changes

dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Jun 5, 2024

eyurtsev changed the title ~~Add SQL docstore~~ community[minor]: Add SQL storage implementation Jun 5, 2024

eyurtsev reviewed Jun 5, 2024

View reviewed changes

eyurtsev requested changes Jun 5, 2024

View reviewed changes

dosubot bot removed the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Jun 5, 2024

pprados and others added 8 commits June 7, 2024 12:31

Add SQL Docstore

47905af

core[patch], community[patch], langchain[patch], docs: Update SQL cha…

e80063d

…ins/agents/docs (langchain-ai#16168) Revamp SQL use cases docs. In the process update SQL chains and agents.

docs: add milvus multitenancy doc (langchain-ai#16177)

c68c818

- **Description:** add milvus multitenancy doc, it is an example for this [pr](langchain-ai#15740) . - **Issue:** No, - **Dependencies:** No, - **Twitter handle:** No Signed-off-by: ChengZi <[email protected]>

Add sql docstore

2b3efff

Add unit test

5f06ce3

Revert changes in tests_dependencies

e8f1f57

Update markers so aiosqlite is handled as an optional

2612714

pprados force-pushed the pprados/new_sql_docstore branch from beb3278 to 2612714 Compare June 7, 2024 10:34

dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Jun 7, 2024

pprados marked this pull request as draft June 7, 2024 10:49

pprados added 4 commits June 7, 2024 13:12

Add unit test

d06ff90

Rename Value()

2e12455

Fix test_dependencies

21e7558

Merge branch 'master' into pprados/new_sql_docstore

37db1c8

vercel bot deployed to Preview June 7, 2024 11:37 View deployment

pprados marked this pull request as ready for review June 7, 2024 11:38

pprados mentioned this pull request Jun 7, 2024

langchain[minor]: Add pgvector to list of supported vectorstores in self query retriever #22678

Merged

2 tasks

Merge branch 'master' into pprados/new_sql_docstore

c432cef

vercel bot deployed to Preview June 7, 2024 15:42 View deployment

eyurtsev added 3 commits June 7, 2024 16:59

Merge branch 'master' into pprados/new_sql_docstore

5b6dc14

x

6cea005

Remove from test dependencies

d95bf3d

eyurtsev approved these changes Jun 7, 2024

View reviewed changes

dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Jun 7, 2024

eyurtsev enabled auto-merge (squash) June 7, 2024 21:04

eyurtsev reviewed Jun 7, 2024

View reviewed changes

vercel bot deployed to Preview June 7, 2024 21:17 View deployment

eyurtsev merged commit 9aabb44 into langchain-ai:master Jun 7, 2024
44 checks passed

pprados deleted the pprados/new_sql_docstore branch June 18, 2024 06:34

efriis mentioned this pull request Jul 19, 2024

infra: add min version testing to pr test flow #24358

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

community[minor]: Add SQL storage implementation #22207

community[minor]: Add SQL storage implementation #22207

pprados commented May 27, 2024

vercel bot commented May 27, 2024 •

edited

Loading

eyurtsev left a comment

pprados commented Jun 7, 2024

eyurtsev Jun 7, 2024

eyurtsev Jun 7, 2024

eyurtsev Jun 7, 2024

		@@ -0,0 +1,186 @@
		"""Implement integration tests for Redis storage."""

community[minor]: Add SQL storage implementation #22207

community[minor]: Add SQL storage implementation #22207

Conversation

pprados commented May 27, 2024

vercel bot commented May 27, 2024 • edited Loading

eyurtsev left a comment

Choose a reason for hiding this comment

pprados commented Jun 7, 2024

eyurtsev Jun 7, 2024

Choose a reason for hiding this comment

eyurtsev Jun 7, 2024

Choose a reason for hiding this comment

eyurtsev Jun 7, 2024

Choose a reason for hiding this comment

vercel bot commented May 27, 2024 •

edited

Loading