feat: add checkpoint aput method and test by carloszuag · Pull Request #269 · googleapis/langchain-google-cloud-sql-pg-python

carloszuag · 2025-02-14T22:49:24Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

averikitsch · 2025-02-14T23:47:54Z

/gcbrun

averikitsch · 2025-02-19T17:33:36Z

/gcbrun

averikitsch · 2025-02-19T17:44:45Z

src/langchain_google_cloud_sql_pg/engine.py

@@ -754,14 +753,14 @@ async def _ainit_checkpoint_table(
        self,


Can we make sure this is the exact same as the AlloyDB library? For example the arguments are in a different order https://github.com/googleapis/langchain-google-alloydb-pg-python/blob/4f24eea67ff0aeec547463f3a77a07f6da209718/src/langchain_google_alloydb_pg/engine.py#L770
and missing quotes around the table name https://github.com/googleapis/langchain-google-alloydb-pg-python/blob/4f24eea67ff0aeec547463f3a77a07f6da209718/src/langchain_google_alloydb_pg/engine.py#L783. please make sure there are no other difference beyond naming differences

Yes, it is the same implementation of the method but, you are right. There was a mismatch in the arguments. It's fixed now.

averikitsch · 2025-02-19T17:47:20Z

src/langchain_google_cloud_sql_pg/async_checkpoint.py

+        self.schema_name = schema_name
+
+    @classmethod
+    async def create(


Please make sure the argument order matches AlloyDB

idem, there was a mismatching. It's addressed.

averikitsch · 2025-02-19T17:48:13Z

tests/test_async_checkpoint.py

+    assert dict(next_config) == test_config
+
+    # Verify if the checkpoint is stored correctly in the database
+    results = await afetch(async_engine, f"SELECT * FROM {table_name}")


Table names need to be in quotes

I have added too all table names the quotes.

averikitsch · 2025-02-19T17:48:30Z

tests/test_async_checkpoint.py

+    yield async_engine
+    # use default table for AsyncPostgresSaver.
+    await aexecute(async_engine, f'DROP TABLE IF EXISTS "{table_name}"')
+    await aexecute(async_engine, f'DROP TABLE IF EXISTS"{table_name_writes}"')


missing space

averikitsch · 2025-02-19T17:50:18Z

tests/test_async_checkpoint.py

+
+project_id = os.environ["PROJECT_ID"]
+region = os.environ["REGION"]
+cluster_id = os.environ["CLUSTER_ID"]


averikitsch · 2025-02-19T17:50:24Z

tests/test_async_checkpoint.py

+    async_engine = await PostgresEngine.afrom_instance(
+        project_id=project_id,
+        region=region,
+        cluster=cluster_id,


averikitsch · 2025-02-19T17:50:28Z

tests/test_async_checkpoint.py

+    return result_fetch
+
+
+@pytest_asyncio.fixture  ##(scope="module")


remove comment

Removed. There was another comment just like that and, I have also removed it.

averikitsch · 2025-02-19T19:58:24Z

/gcbrun

averikitsch · 2025-02-19T20:06:25Z

/usr/local/lib/python3.9/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/test_engine.py:31: in <module>
    from langchain_google_cloud_sql_pg.engine import (
E   ImportError: cannot import name 'CHECKPOINT_WRITES_TABLE' from 'langchain_google_cloud_sql_pg.engine' (/builder/home/.local/lib/python3.9/site-packages/langchain_google_cloud_sql_pg/engine.py)

averikitsch · 2025-02-21T19:19:34Z

/gcbrun

tests/test_async_checkpoint.py

averikitsch · 2025-02-24T19:32:36Z

/gcbrun

averikitsch · 2025-02-24T19:42:52Z

src/langchain_google_cloud_sql_pg/engine.py

            )

-        return metadata.tables[f"{schema_name}.{table_name}"]
+        return metadata.tables[f'"{schema_name}"."{table_name}"']


remove change

averikitsch · 2025-02-24T19:44:35Z

tests/test_async_checkpoint.py

+    await aexecute(async_engine, f'DROP TABLE IF EXISTS "{table_name}"')
+    await aexecute(async_engine, f'DROP TABLE IF EXISTS "{table_name_writes}"')
+    await async_engine.close()
+    await async_engine._connector.close()


Use .close_async()

I have a doubt here, do I need to use await "async_engine.close_async()" instead of "await async_engine.close()"?

The for the connector use close_async() for the engine continue to use close(). https://github.com/GoogleCloudPlatform/cloud-sql-python-connector?tab=readme-ov-file#async-context-manager

I changed this and I added the "async_engine.close_asyn()" just as you suggested.

averikitsch · 2025-02-25T23:00:50Z

/gcbrun

averikitsch · 2025-02-25T23:13:32Z

tests/test_engine.py

-
-        # Verify that the query is executed on the custom table.
-        stmt = f"SELECT column_name, data_type FROM information_schema.columns WHERE table_name = '{custom_table_name}';"
+        stmt = f'SELECT column_name, data_type FROM information_schema.columns WHERE table_name = "{table_name}";'


These values shouldn't be in double quotes only single quotes

I've removes double quotes here.

averikitsch

____________________________ test_checkpoint_alist _____________________________

async_engine = <langchain_google_cloud_sql_pg.engine.PostgresEngine object at 0x7f8ed8342310>
checkpointer = <langchain_google_cloud_sql_pg.async_checkpoint.AsyncPostgresSaver object at 0x7f8ed83ffbd0>
test_data = {'checkpoints': [{'channel_values': {'my_key': 'meow', 'node': 'node'}, 'channel_versions': {'start': 2, 'my_key':...rce': 'input', 'step': 2, 'writes': {}}, {'parents': None, 'source': 'loop', 'step': 1, 'writes': {'foo': 'bar'}}, {}]}

@pytest.mark.asyncio
async def test_checkpoint_alist(
    async_engine: PostgresEngine,
    checkpointer: AsyncPostgresSaver,
    test_data: dict[str, Any],
) -> None:
    configs = test_data["configs"]
    checkpoints = test_data["checkpoints"]
    metadata = test_data["metadata"]

    await checkpointer.aput(configs[1], checkpoints[1], metadata[0], {})
    await checkpointer.aput(configs[2], checkpoints[2], metadata[1], {})
    await checkpointer.aput(configs[3], checkpoints[3], metadata[2], {})

    # call method / assertions
    query_1 = {"source": "input"}  # search by 1 key
    query_2 = {
        "step": 1,
        "writes": {"foo": "bar"},
    }  # search by multiple keys
    query_3: dict[str, Any] = {}  # search by no keys, return all checkpoints
    query_4 = {"source": "update", "step": 1}  # no match

  search_results_1 = [c async for c in checkpointer.alist(None, filter=query_1)]

tests/test_async_checkpoint.py:242:

tests/test_async_checkpoint.py:242: in
search_results_1 = [c async for c in checkpointer.alist(None, filter=query_1)]
/builder/home/.local/lib/python3.11/site-packages/langchain_google_cloud_sql_pg/async_checkpoint.py:444: in alist
result = await conn.stream(text(query), args)
/builder/home/.local/lib/python3.11/site-packages/sqlalchemy/ext/asyncio/base.py:151: in start
start_value = await util.anext_(self.gen)
/builder/home/.local/lib/python3.11/site-packages/sqlalchemy/ext/asyncio/engine.py:582: in stream
result = await greenlet_spawn(
/builder/home/.local/lib/python3.11/site-packages/sqlalchemy/util/_concurrency_py3k.py:190: in greenlet_spawn
result = context.switch(*args, **kwargs)
/builder/home/.local/lib/python3.11/site-packages/sqlalchemy/engine/base.py:1412: in execute
distilled_parameters = _distill_params_20(parameters)
lib/sqlalchemy/cyextension/util.pyx:32: in sqlalchemy.cyextension.util._distill_params_20
???

???
E sqlalchemy.exc.ArgumentError: List argument must consist only of tuples or dictionaries

lib/sqlalchemy/cyextension/util.pyx:23: ArgumentError

averikitsch · 2025-02-26T22:01:01Z

/gcbrun

tests/test_engine.py

averikitsch · 2025-02-26T22:17:38Z

tests/test_engine.py

-
-        # Verify that the query is executed on the custom table.
-        stmt = f"SELECT column_name, data_type FROM information_schema.columns WHERE table_name = '{custom_table_name}';"
+        stmt = f'SELECT column_name, data_type FROM information_schema.columns WHERE table_name = "{table_name_writes}";'


please update to use single quotes around table name

Ok, I've replaced the single quotes for double quotes instead.

averikitsch · 2025-02-26T22:17:47Z

tests/test_engine.py

+        table_name = f"checkpoint{uuid.uuid4()}"
+        table_name_writes = f"{table_name}_writes"
+        await engine.ainit_checkpoint_table(table_name=table_name)
+        stmt = f'SELECT column_name, data_type FROM information_schema.columns WHERE table_name = "{table_name_writes}";'


please update to use single quotes around table name

This was addressed.

averikitsch · 2025-02-26T22:17:55Z

tests/test_engine.py

+        table_name = f"checkpoint{uuid.uuid4()}"
+        table_name_writes = f"{table_name}_writes"
+        engine.init_checkpoint_table(table_name=table_name)
+        stmt = f'SELECT column_name, data_type FROM information_schema.columns WHERE table_name = "{table_name}";'


please update to use single quotes around table name

This was also addressed. Thank you for all the comments.

averikitsch · 2025-03-05T00:51:59Z

/gcbrun

feat : create checkpoint aput method and test

ca01f7c

carloszuag requested review from a team as code owners February 14, 2025 22:49

blunderbuss-gcf bot assigned averikitsch Feb 14, 2025

product-auto-label bot added the api: cloudsql-postgres Issues related to the googleapis/langchain-google-cloud-sql-pg-python API. label Feb 14, 2025

carloszuag changed the title ~~feat : create checkpoint aput method and test~~ feat: add checkpoint aput method and test Feb 14, 2025

adding aput method

d8f87da

Carlos Zúñiga Aguilar added 2 commits February 18, 2025 16:56

adding write method for langraph-base-aput

9ef71a2

adding alist method for langraph-base-aput

58c1c7e

averikitsch requested changes Feb 19, 2025

View reviewed changes

addressing comments in PR

eb86648

Carlos Zúñiga Aguilar added 2 commits February 21, 2025 18:14

adding missed variables

205b002

adding quotes in some table names

23bc89f

This was referenced Feb 24, 2025

feat: add alist method #264

Closed

feat: add write method #263

Closed

feat: add checkpoint postgresql #262

Closed

Feat: add checkpointer class implementation #266

Closed

averikitsch reviewed Feb 24, 2025

View reviewed changes

tests/test_async_checkpoint.py Outdated Show resolved Hide resolved

Update tests/test_async_checkpoint.py

06a7003

averikitsch reviewed Feb 24, 2025

View reviewed changes

tests/test_async_checkpoint.py Show resolved Hide resolved

Update tests/test_async_checkpoint.py

e76dd16

averikitsch reviewed Feb 24, 2025

View reviewed changes

addressing comments on closing engine

d028cf6

averikitsch reviewed Feb 25, 2025

View reviewed changes

fixing alist method

e593a96

averikitsch reviewed Feb 26, 2025

View reviewed changes

tests/test_engine.py Show resolved Hide resolved

averikitsch reviewed Feb 26, 2025

View reviewed changes

Carlos Zúñiga Aguilar added 2 commits March 4, 2025 20:56

addresing comments and fixing tests

96212ae

Apply code formatting

7807ec9

averikitsch approved these changes Mar 5, 2025

View reviewed changes

averikitsch merged commit ea22dd0 into googleapis:langgraph-base Mar 5, 2025
4 of 8 checks passed

		@@ -754,14 +753,14 @@ async def _ainit_checkpoint_table(
		self,

		return result_fetch


		@pytest_asyncio.fixture ##(scope="module")

Comments

Conversation

carloszuag commented Feb 14, 2025

Uh oh!

averikitsch commented Feb 14, 2025

Uh oh!

averikitsch commented Feb 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carloszuag Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

averikitsch commented Feb 19, 2025

Uh oh!

averikitsch commented Feb 19, 2025

Uh oh!

averikitsch commented Feb 21, 2025

Uh oh!

Uh oh!

Uh oh!

averikitsch commented Feb 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

averikitsch commented Feb 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

averikitsch left a comment

Choose a reason for hiding this comment

Uh oh!

averikitsch commented Feb 26, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carloszuag Feb 19, 2025 •

edited

Loading