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

feat: Added manually tested sync methods for PGVector to AlloyDB migration #209

Closed
wants to merge 7 commits into from

Conversation

twishabansal
Copy link
Contributor

No description provided.

@product-auto-label product-auto-label bot added the api: alloydb Issues related to the googleapis/langchain-google-alloydb-pg-python API. label Aug 27, 2024
@twishabansal twishabansal changed the title Added manually tested sync methods for PGVector to AlloyDB migration feat: Added manually tested sync methods for PGVector to AlloyDB migration Aug 27, 2024
@twishabansal twishabansal force-pushed the migration_methods branch 6 times, most recently from 502d2b8 to e5ae00a Compare August 27, 2024 07:26
src/langchain_google_alloydb_pg/engine.py Outdated Show resolved Hide resolved
src/langchain_google_alloydb_pg/engine.py Outdated Show resolved Hide resolved
params = {}

if use_json_metadata:
insert_query = f"INSERT INTO {destination_table} (LANGCHAIN_ID, CONTENT, EMBEDDING, LANGCHAIN_METADATA) VALUES"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We get these default values from the file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see the default values hardcoded here:

Which file can we extract them from?

src/langchain_google_alloydb_pg/engine.py Outdated Show resolved Hide resolved
The uuid corresponding to the collection.
"""
try:
collection_info = self._fetch(
Copy link
Collaborator

Choose a reason for hiding this comment

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

The upcoming refactor we will be deleting fetch/execute. Any where that you use fetch or execute use the async pool directly like async with self.engine.connect() as conn:.... Then we will wrap the async methods with sync wrappers. For example extract_pgvector_collection could be _aextract_pgvector_collection. Then in extract_pgvector_collection we will use run_as_sync to run the _aextract_pgvector_collection methods. See https://github.com/googleapis/langchain-google-cloud-sql-pg-python/blob/c3a0f12276eef07a64ea40b20abe496496b5132c/src/langchain_google_cloud_sql_pg/engine.py#L364 for an example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll refactor this PR once the async/sync refactor is pushed.

@twishabansal twishabansal force-pushed the migration_methods branch 2 times, most recently from 630dd4f to c679a14 Compare September 2, 2024 09:20
@averikitsch
Copy link
Collaborator

@twishabansal This PR can now be closed in favor of #235, correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: alloydb Issues related to the googleapis/langchain-google-alloydb-pg-python API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants