Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 7 additions & 6 deletions src/google/adk/sessions/database_session_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
_MARIADB_DIALECT = "mariadb"
_MYSQL_DIALECT = "mysql"
_POSTGRESQL_DIALECT = "postgresql"
_DATABRICKS_DIALECT = "databricks"
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The newly added constant _DATABRICKS_DIALECT does not appear to be used anywhere in the project. If it's not intended for immediate use in a subsequent change, it should be removed to keep the codebase clean.

# Tuple key order for in-process per-session lock maps:
# (app_name, user_id, session_id).
_SessionLockKey: TypeAlias = tuple[str, str, str]
Expand Down Expand Up @@ -627,12 +628,12 @@ async def append_event(self, session: Session, event: Event) -> Event:
if session_state_delta:
storage_session.state = storage_session.state | session_state_delta

if is_sqlite:
update_time = datetime.fromtimestamp(
event.timestamp, timezone.utc
).replace(tzinfo=None)
else:
update_time = datetime.fromtimestamp(event.timestamp)
# Convert event timestamp to a UTC datetime. SQLite and PostgreSQL
# use TIMESTAMP WITHOUT TIME ZONE, so the tzinfo must be stripped to
# avoid asyncpg DataError / SQLite comparison issues.
update_time = datetime.fromtimestamp(event.timestamp, timezone.utc)
if is_sqlite or self.db_engine.dialect.name == _POSTGRESQL_DIALECT:
update_time = update_time.replace(tzinfo=None)
storage_session.update_time = update_time
sql_session.add(schema.StorageEvent.from_event(session, event))

Expand Down
15 changes: 9 additions & 6 deletions src/google/adk/sessions/schemas/v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,15 @@ def update_timestamp_tz(self) -> float:
)
return self.get_update_timestamp(is_sqlite=is_sqlite)

def get_update_timestamp(self, is_sqlite: bool) -> float:
"""Returns the time zone aware update timestamp."""
if is_sqlite:
# SQLite does not support timezone. SQLAlchemy returns a naive datetime
# object without timezone information. We need to convert it to UTC
# manually.
def get_update_timestamp(self, is_sqlite: bool = False) -> float:
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The is_sqlite parameter is no longer used within this function's body. The new implementation correctly relies on checking self.update_time.tzinfo is None, which is more robust. To improve code quality, this parameter should be removed from the function signature. This would require updating all call sites. If modifying all call sites is outside the scope of this PR, consider adding a TODO comment to track this for future refactoring.

"""Returns the update timestamp as a POSIX float.

Naive datetimes (returned by SQLite and PostgreSQL which use
``TIMESTAMP WITHOUT TIME ZONE``) are interpreted as UTC. Timezone-aware
datetimes (MySQL, MariaDB) are used directly.
"""
if self.update_time.tzinfo is None:
# Naive datetimes from SQLite / PostgreSQL are stored as UTC.
return self.update_time.replace(tzinfo=timezone.utc).timestamp()
return self.update_time.timestamp()

Expand Down