Skip to content

Commit 2f799d5

Browse files
haranrkcopybara-github
authored andcommitted
fix(sessions): strip tzinfo for MariaDB in DatabaseSessionService
MariaDB stores DATETIME/TIMESTAMP values without timezone information, exactly like MySQL. create_session previously produced a timezone-aware _storage_update_marker for MariaDB, but the value read back from storage is naive, so the strict marker comparison in append_event raised a false "The session has been modified in storage" ValueError on the first append_event after create_session. Add MariaDB to the set of dialects whose datetimes are stored as naive, and consolidate the timezone-naive-dialect decision into a single _NAIVE_DATETIME_DIALECTS constant and a _uses_naive_datetime() helper used by create_session, so the strip logic lives in one place. Cloud Spanner is intentionally excluded because its TIMESTAMP is timezone-aware. Co-authored-by: Haran Rajkumar <haranrk@google.com> PiperOrigin-RevId: 937063102
1 parent ea2ea73 commit 2f799d5

2 files changed

Lines changed: 50 additions & 21 deletions

File tree

src/google/adk/sessions/database_session_service.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,18 @@
7979
_MARIADB_DIALECT = "mariadb"
8080
_MYSQL_DIALECT = "mysql"
8181
_POSTGRESQL_DIALECT = "postgresql"
82+
# Dialects whose DATETIME/TIMESTAMP columns do not retain timezone info, so
83+
# timezone-aware datetimes must have their tzinfo stripped before storage. This
84+
# keeps the value written by create_session consistent with the value read back
85+
# from storage; otherwise the stale-writer marker comparison in append_event
86+
# raises a false positive on the first append after create_session. Cloud
87+
# Spanner is intentionally excluded because its TIMESTAMP is timezone-aware.
88+
_NAIVE_DATETIME_DIALECTS = (
89+
_SQLITE_DIALECT,
90+
_POSTGRESQL_DIALECT,
91+
_MYSQL_DIALECT,
92+
_MARIADB_DIALECT,
93+
)
8294
# Tuple key order for in-process per-session lock maps:
8395
# (app_name, user_id, session_id).
8496
_SessionLockKey: TypeAlias = tuple[str, str, str]
@@ -348,6 +360,14 @@ def _supports_row_level_locking(self) -> bool:
348360
_POSTGRESQL_DIALECT,
349361
)
350362

363+
def _uses_naive_datetime(self) -> bool:
364+
"""Returns whether the active dialect stores datetimes without timezone info.
365+
366+
These dialects persist timezone-naive DATETIME/TIMESTAMP values, so
367+
timezone-aware datetimes must have their tzinfo stripped before storage.
368+
"""
369+
return self.db_engine.dialect.name in _NAIVE_DATETIME_DIALECTS
370+
351371
@asynccontextmanager
352372
async def _with_session_lock(
353373
self, *, app_name: str, user_id: str, session_id: str
@@ -531,8 +551,7 @@ async def create_session(
531551
now = datetime.fromtimestamp(platform_time.get_time(), tz=timezone.utc)
532552
is_sqlite = self.db_engine.dialect.name == _SQLITE_DIALECT
533553
is_postgresql = self.db_engine.dialect.name == _POSTGRESQL_DIALECT
534-
is_mysql = self.db_engine.dialect.name == _MYSQL_DIALECT
535-
if is_sqlite or is_postgresql or is_mysql:
554+
if self._uses_naive_datetime():
536555
now = now.replace(tzinfo=None)
537556

538557
storage_session = schema.StorageSession(

tests/unittests/sessions/test_session_service.py

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -106,29 +106,39 @@ def fake_create_async_engine(_db_url: str, **kwargs):
106106
assert captured_kwargs.get('pool_pre_ping') is True
107107

108108

109-
@pytest.mark.parametrize('dialect_name', ['sqlite', 'postgresql', 'mysql'])
110-
def test_database_session_service_strips_timezone_for_dialect(dialect_name):
111-
"""Verifies that timezone-aware datetimes are converted to naive datetimes
112-
for SQLite, PostgreSQL, and MySQL.
113-
114-
These databases store DATETIME/TIMESTAMP WITHOUT TIME ZONE, so keeping
115-
tzinfo on the Python datetime causes a mismatch between the marker
116-
produced by create_session (with +00:00) and the marker read back by
117-
get_session (without +00:00), triggering a false stale-writer error
118-
on the first append_event after create_session.
109+
@pytest.mark.parametrize(
110+
'dialect_name', ['sqlite', 'postgresql', 'mysql', 'mariadb']
111+
)
112+
def test_database_session_service_uses_naive_datetime_for_dialect(dialect_name):
113+
"""Verifies dialects that store DATETIME WITHOUT TIME ZONE are treated as naive.
114+
115+
SQLite, PostgreSQL, MySQL, and MariaDB all store DATETIME/TIMESTAMP WITHOUT
116+
TIME ZONE, so create_session must strip tzinfo before storing. Otherwise the
117+
marker produced by create_session (with +00:00) mismatches the marker read
118+
back from storage (without +00:00), triggering a false stale-writer error on
119+
the first append_event after create_session.
120+
121+
This exercises the production decision (_uses_naive_datetime) directly rather
122+
than re-implementing the strip logic, so it actually guards create_session.
119123
"""
120-
# Simulate the logic in create_session
121-
is_sqlite = dialect_name == 'sqlite'
122-
is_postgres = dialect_name == 'postgresql'
123-
is_mysql = dialect_name == 'mysql'
124+
fake_engine = mock.Mock()
125+
fake_engine.dialect.name = dialect_name
126+
fake_engine.sync_engine = mock.Mock()
124127

125-
now = datetime.now(timezone.utc)
126-
assert now.tzinfo is not None # Starts with timezone
128+
service = DatabaseSessionService(db_engine=fake_engine)
129+
130+
assert service._uses_naive_datetime() is True
131+
132+
133+
def test_database_session_service_keeps_timezone_for_spanner():
134+
"""Spanner's TIMESTAMP is timezone-aware, so tzinfo must be preserved."""
135+
fake_engine = mock.Mock()
136+
fake_engine.dialect.name = 'spanner+spanner'
137+
fake_engine.sync_engine = mock.Mock()
127138

128-
if is_sqlite or is_postgres or is_mysql:
129-
now = now.replace(tzinfo=None)
139+
service = DatabaseSessionService(db_engine=fake_engine)
130140

131-
assert now.tzinfo is None
141+
assert service._uses_naive_datetime() is False
132142

133143

134144
def test_database_session_service_respects_pool_pre_ping_override():

0 commit comments

Comments
 (0)