Skip to content

Commit 4e133b7

Browse files
fix(server): create the cluster tables lazily in VersionedDatabaseTaskStore get and delete (#1357)
# Description 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: - [x] Follow the [`CONTRIBUTING` Guide](https://github.com/a2aproject/a2a-python/blob/main/CONTRIBUTING.md). - [x] Make your Pull Request title in the <https://www.conventionalcommits.org/> specification. - Important Prefixes for [release-please](https://github.com/googleapis/release-please): - `fix:` which represents bug fixes, and correlates to a [SemVer](https://semver.org/) patch. - `feat:` represents a new feature, and correlates to a SemVer minor. - `feat!:`, or `fix!:`, `refactor!:`, etc., which represent a breaking change (indicated by the `!`) and will result in a SemVer major. - [x] Ensure the tests and linter pass (Run `bash scripts/format.sh` from the repository root to format) - [x] Appropriate docs were updated (if necessary) Fixes #1356 🦕 ## Problem `VersionedDatabaseTaskStore` creates its tables lazily, like `DatabaseTaskStore`, but only `save()` creates the cluster tables. `get()` and `delete()` initialize the underlying `DatabaseTaskStore` only, so when the store is used without calling `initialize()` and the first operation is a `get()` or a `delete()`, the query on `task_versions` fails: ```python store = VersionedDatabaseTaskStore(engine=create_async_engine('sqlite+aiosqlite:///:memory:')) await store.get('missing', ServerCallContext()) # sqlalchemy.exc.OperationalError: (sqlite3.OperationalError) no such table: task_versions ``` ## Fix `get()` and `delete()` (`src/a2a/server/cluster/database_task_store.py`) now call `self._ensure_cluster_tables()` after initializing the underlying store, the same way `save()` does. `delete()` also initializes the underlying store before creating the cluster tables, so the tables are created in the same order as in `save()`. `_ensure_cluster_tables()` returns right away once the tables are ready, so stores that call `initialize()` are not affected. ## Test Tests were added to the existing suite at `tests/server/cluster/test_database_versioned_task_store.py`, using a new `uninitialized_store` fixture that builds the store on an empty database with `create_table=True` and without calling `initialize()`: - `test_get_without_initialize_creates_tables`: `get()` of a missing task returns `None`. - `test_delete_without_initialize_creates_tables`: `delete()` of a missing task does not fail, and a later `get()` returns `None`. Both tests fail on `main` with `no such table: task_versions` and pass with the fix. No existing test was modified or removed. Happy to adapt the test approach based on maintainer feedback. Co-authored-by: mykytanetipa <mykytanetipa@google.com>
1 parent babc3f5 commit 4e133b7

2 files changed

Lines changed: 36 additions & 0 deletions

File tree

‎src/a2a/server/cluster/database_task_store.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,7 @@ async def get(
258258
commits) rather than failing the read.
259259
"""
260260
await self._db._ensure_initialized() # noqa: SLF001
261+
await self._ensure_cluster_tables()
261262
owner = self._db.owner_resolver(context)
262263
attempts = 0
263264
while True:
@@ -355,6 +356,8 @@ async def list(
355356

356357
async def delete(self, task_id: str, context: ServerCallContext) -> None:
357358
"""Deletes a task and its version row via the underlying store."""
359+
await self._db._ensure_initialized() # noqa: SLF001
360+
await self._ensure_cluster_tables()
358361
owner = self._db.owner_resolver(context)
359362
await self._db.delete(task_id, context)
360363
async with self._db.async_session_maker.begin() as session:

‎tests/server/cluster/test_database_versioned_task_store.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,24 @@ async def versioned_store(
136136
await engine.dispose()
137137

138138

139+
@pytest_asyncio.fixture(params=DB_CONFIGS)
140+
async def uninitialized_store(
141+
request,
142+
) -> AsyncGenerator[VersionedDatabaseTaskStore, None]:
143+
"""A store that creates its tables lazily, without `initialize()`."""
144+
db_url, dialect_name = request.param
145+
if db_url is None:
146+
pytest.skip(f'DSN for {dialect_name} not set in environment variables.')
147+
engine = create_async_engine(db_url)
148+
_enforce_sqlite_foreign_keys(engine)
149+
try:
150+
yield VersionedDatabaseTaskStore(engine=engine, create_table=True)
151+
finally:
152+
async with engine.begin() as conn:
153+
await conn.run_sync(Base.metadata.drop_all)
154+
await engine.dispose()
155+
156+
139157
@pytest.mark.asyncio
140158
async def test_is_a_versioned_task_store(
141159
versioned_store: VersionedDatabaseTaskStore,
@@ -654,6 +672,21 @@ async def test_delete_removes_version_row(
654672
assert stored is not None
655673

656674

675+
@pytest.mark.asyncio
676+
async def test_get_without_initialize_creates_tables(
677+
uninitialized_store: VersionedDatabaseTaskStore,
678+
) -> None:
679+
assert await uninitialized_store.get('task-abc', TEST_CONTEXT) is None
680+
681+
682+
@pytest.mark.asyncio
683+
async def test_delete_without_initialize_creates_tables(
684+
uninitialized_store: VersionedDatabaseTaskStore,
685+
) -> None:
686+
await uninitialized_store.delete('task-abc', TEST_CONTEXT)
687+
assert await uninitialized_store.get('task-abc', TEST_CONTEXT) is None
688+
689+
657690
@pytest.mark.asyncio
658691
async def test_versions_count_writes(
659692
versioned_store: VersionedDatabaseTaskStore,

0 commit comments

Comments
 (0)