Skip to content

Commit a012bb7

Browse files
andreikravchenko-ovivaharanrk
authored andcommitted
fix: strip tzinfo from datetime for MySQL in create_session
Merge google#5084 ### Link to Issue or Description of Change Fixes google#5085 When using `DatabaseSessionService` with MySQL, the very first `append_event()` after `create_session()` always fails with: ``` ValueError: The session has been modified in storage since it was loaded. Please reload the session before appending more events. ``` **Root cause:** `create_session()` produces a timezone-aware `_storage_update_marker` (with `+00:00`), but MySQL `DATETIME(fsp=6)` doesn't store timezone info ([MySQL docs](https://dev.mysql.com/doc/refman/8.0/en/datetime.html)). When `append_event()` reads `update_time` back, it gets a naive datetime — different marker string, strict comparison fails. The code already strips `tzinfo` for SQLite and PostgreSQL but MySQL was missed: ```python # Before (line 464): if is_sqlite or is_postgresql: # MySQL not included now = now.replace(tzinfo=None) # After: is_mysql = self.db_engine.dialect.name == _MYSQL_DIALECT if is_sqlite or is_postgresql or is_mysql: now = now.replace(tzinfo=None) ``` ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. Added `mysql` to the existing parametrized dialect test. Removed the test that incorrectly asserted MySQL preserves timezone. **Manual End-to-End (E2E) Tests:** Verified marker values with Testcontainers MySQL 8.0: | | create_session marker | get_session marker | Match? | |---|---|---|---| | Before fix | `...T14:39:46.014977+00:00` | `...T14:39:46.014977` | No | | After fix | `...T14:44:49.388273` | `...T14:44:49.388273` | Yes | Confirmed that `append_event()` works immediately after `create_session()` without needing a re-fetch workaround. ### Checklist - [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [x] Any dependent changes have been merged and published in downstream modules. Co-authored-by: Haran Rajkumar <haranrk@google.com> COPYBARA_INTEGRATE_REVIEW=google#5084 from andreikravchenko-oviva:fix/mysql-stale-session-marker 30bccf1 PiperOrigin-RevId: 934446269
1 parent 066fbce commit a012bb7

2 files changed

Lines changed: 11 additions & 26 deletions

File tree

src/google/adk/sessions/database_session_service.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,8 @@ async def create_session(
531531
now = datetime.fromtimestamp(platform_time.get_time(), tz=timezone.utc)
532532
is_sqlite = self.db_engine.dialect.name == _SQLITE_DIALECT
533533
is_postgresql = self.db_engine.dialect.name == _POSTGRESQL_DIALECT
534-
if is_sqlite or is_postgresql:
534+
is_mysql = self.db_engine.dialect.name == _MYSQL_DIALECT
535+
if is_sqlite or is_postgresql or is_mysql:
535536
now = now.replace(tzinfo=None)
536537

537538
storage_session = schema.StorageSession(

tests/unittests/sessions/test_session_service.py

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -106,47 +106,31 @@ 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'])
109+
@pytest.mark.parametrize('dialect_name', ['sqlite', 'postgresql', 'mysql'])
110110
def test_database_session_service_strips_timezone_for_dialect(dialect_name):
111111
"""Verifies that timezone-aware datetimes are converted to naive datetimes
112-
for SQLite and PostgreSQL to avoid 'can't subtract offset-naive and
113-
offset-aware datetimes' errors.
112+
for SQLite, PostgreSQL, and MySQL.
114113
115-
PostgreSQL's default TIMESTAMP type is WITHOUT TIME ZONE, which cannot
116-
accept timezone-aware datetime objects when using asyncpg. SQLite also
117-
requires naive datetimes.
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.
118119
"""
119120
# Simulate the logic in create_session
120121
is_sqlite = dialect_name == 'sqlite'
121122
is_postgres = dialect_name == 'postgresql'
123+
is_mysql = dialect_name == 'mysql'
122124

123125
now = datetime.now(timezone.utc)
124126
assert now.tzinfo is not None # Starts with timezone
125127

126-
if is_sqlite or is_postgres:
128+
if is_sqlite or is_postgres or is_mysql:
127129
now = now.replace(tzinfo=None)
128130

129-
# Both SQLite and PostgreSQL should have timezone stripped
130131
assert now.tzinfo is None
131132

132133

133-
def test_database_session_service_preserves_timezone_for_other_dialects():
134-
"""Verifies that timezone info is preserved for dialects that support it."""
135-
# For dialects like MySQL with explicit timezone support, we don't strip
136-
dialect_name = 'mysql'
137-
is_sqlite = dialect_name == 'sqlite'
138-
is_postgres = dialect_name == 'postgresql'
139-
140-
now = datetime.now(timezone.utc)
141-
assert now.tzinfo is not None
142-
143-
if is_sqlite or is_postgres:
144-
now = now.replace(tzinfo=None)
145-
146-
# MySQL should preserve timezone (if the column type supports it)
147-
assert now.tzinfo is not None
148-
149-
150134
def test_database_session_service_respects_pool_pre_ping_override():
151135
captured_kwargs = {}
152136

0 commit comments

Comments
 (0)