|
16 | 16 |
|
17 | 17 | import logging |
18 | 18 | import os |
| 19 | +from datetime import datetime |
| 20 | +from datetime import timezone |
19 | 21 | from typing import Any |
20 | 22 | from typing import Optional |
21 | 23 |
|
@@ -114,9 +116,6 @@ async def create_session( |
114 | 116 | # evaluated on the server, we might want to use local time for the object |
115 | 117 | # or read it back. Reading it back is expensive. We'll use local time for |
116 | 118 | # the object, but the DB will have SERVER_TIMESTAMP. |
117 | | - from datetime import datetime |
118 | | - from datetime import timezone |
119 | | - |
120 | 119 | local_now = datetime.now(timezone.utc).timestamp() |
121 | 120 |
|
122 | 121 | return Session( |
@@ -170,60 +169,6 @@ async def get_session( |
170 | 169 | # Restore timestamp if needed, or assume it's in event_data |
171 | 170 | events.append(Event.model_validate(ed)) |
172 | 171 |
|
173 | | - # Fetch states (app and user) if we want to merge them, similar to |
174 | | - # DatabaseSessionService. The Java code seems to merge them in listSessions |
175 | | - # but let's see if getSession does it. |
176 | | - # In Java, getSession fetches app/user state if needed? The Java code I read: |
177 | | - # It didn't seem to fetch app/user state in getSession, only in appendEvent |
178 | | - # where it updates them, and listSessions where it mergers. |
179 | | - # Wait, let's re-read Java getSession. |
180 | | - # It doesn't seem to fetch app/user state in getSession either? |
181 | | - # Actually, in Java `FirestoreSessionService.java` `getSession`: |
182 | | - # It reads the session doc, then reads events. It doesn't seem to read |
183 | | - # app/user state docs. |
184 | | - # But `DatabaseSessionService` in Python DOES read them in `get_session`. |
185 | | - # Let's align with Python `DatabaseSessionService` if possible, as it's the |
186 | | - # standard in Python ADK. |
187 | | - # Python `DatabaseSessionService` reads `StorageAppState` and `StorageUserState` |
188 | | - # and merges them. |
189 | | - # If I want to be consistent with Python ADK, I should probably do it. |
190 | | - # But if I want to be consistent with Java ADK port, I should follow Java. |
191 | | - # The user asked to "Port this firestore support over to ADK Python". |
192 | | - # I should follow the Java logic but make it Pythonic. |
193 | | - # The Java logic doesn't seem to merge app/user state in `getSession`, it |
194 | | - # just returns session state. |
195 | | - # Wait, let's check Java `listSessions`. It read `StorageAppState`? No, it |
196 | | - # just read sessions. |
197 | | - # Let's stick to the Java logic if it works, or adapt to Python if it's better. |
198 | | - # Since `DatabaseSessionService` in Python merges them, maybe it's a newer |
199 | | - # feature in Python ADK that Java doesn't have or does differently. |
200 | | - # Let's check `FirestoreSessionService.java` again. |
201 | | - # In Java `listSessions`, it doesn't seem to fetch app/user state. |
202 | | - # In Java `appendEvent`, it updates app/user state if `state_delta` has |
203 | | - # `_app_` or `_user_` prefixes. |
204 | | - # Let's stick to the Java behavior unless it conflicts with Python interfaces. |
205 | | - # The Python `BaseSessionService` doesn't enforce merging, it just defines |
206 | | - # the interface. `DatabaseSessionService` implements merging. |
207 | | - # I'll stick to the Java behavior (no merging in get/list, only update in append) |
208 | | - # for now, as it's a port of Java. Or I can implement merging if it's easy. |
209 | | - # Let's look at Java `appendEvent`: |
210 | | - # It checks `_app_` and `_user_` prefixes in `state_delta` and updates |
211 | | - # separate collections! |
212 | | - # ```java |
213 | | - # firestore.collection(APP_STATE_COLLECTION).document(appName).set(...) |
214 | | - # ``` |
215 | | - # So it DOES use separate collections for app/user state. |
216 | | - # If it uses them, it should probably read them somewhere. In Java, it seems |
217 | | - # it might not read them in `getSession`? Wait, let's check `FirestoreSessionService.java` |
218 | | - # again. I see `listSessions` doesn't read them. `getSession` doesn't read them. |
219 | | - # That might be a bug or partial implementation in Java? Or maybe they are |
220 | | - # read elsewhere? |
221 | | - # In Python `DatabaseSessionService` reads them in `get_session` and `list_sessions`. |
222 | | - # Let's implement reading them in Python `FirestoreSessionService` to be |
223 | | - # consistent with Python ADK standards if possible, or at least support it. |
224 | | - # I'll implement it without merging first to match Java, then see if I should |
225 | | - # add it. The Java code didn't do it. |
226 | | - |
227 | 172 | # Let's continue getting session. |
228 | 173 | session_state = data.get("state", {}) |
229 | 174 |
|
|
0 commit comments