Skip to content

refs #4077, removing sqlite prefix in path to db file expected by aio… #4081

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.

Sign up for GitHub

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

Draft
agherasim wants to merge 1 commit into
base: main
Choose a base branch
from
+3 −1
Draft

refs #4077, removing sqlite prefix in path to db file expected by aio… #4081

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/google/adk/sessions/sqlite_session_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,9 @@ async def append_event(self, session: Session, event: Event) -> Event:
@asynccontextmanager
async def _get_db_connection(self):
"""Connects to the db and performs initial setup."""
async with aiosqlite.connect(self._db_path) as db:
# aiosqlite requires a file path
path = self._db_path.replace("sqlite+aiosqlite:///", "")
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 6, 2026

Choose a reason for hiding this comment

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

medium

Using str.replace() for removing a prefix can be less robust than alternatives. For removing a prefix, it's more idiomatic and safer to use str.removeprefix() (available in Python 3.9+). Given the modern Python features used elsewhere in the codebase, removeprefix is likely available and is the cleanest option.

Suggested change
path = self._db_path.replace("sqlite+aiosqlite:///", "")
path = self._db_path.removeprefix("sqlite+aiosqlite:///")

async with aiosqlite.connect(path) as db:
Comment on lines +418 to +420
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 6, 2026

Choose a reason for hiding this comment

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

high

While this change correctly handles the database path for aiosqlite.connect, a similar issue exists in the __init__ method. The _is_migration_needed method is called from __init__ and it internally uses os.path.exists(self._db_path) and sqlite3.connect(self._db_path). Both of these will fail if self._db_path is a URI like sqlite+aiosqlite:///..., causing a RuntimeError during the initialization of SqliteSessionService before this method is ever called.

To resolve this, the path stripping logic should be applied in __init__ so that self._db_path holds the clean file path throughout the object's lifecycle.

db.row_factory = aiosqlite.Row
await db.execute(PRAGMA_FOREIGN_KEYS)
await db.executescript(CREATE_SCHEMA_SQL)
Expand Down