Conversation
|
Response from ADK Triaging Agent Hello @ScottMansfield, thank you for creating this PR! I see that this is a draft PR. When you are ready, please ensure you have read the contribution guide and filled out the PR template, including:
This information will help reviewers to review your PR more efficiently. Thanks! |
e3f5d6a to
49c7bf5
Compare
|
Looks like we have one open for the community repo: google/adk-python-community#104 |
|
Hi @ScottMansfield , Thank you for your contribution! We appreciate you taking the time to submit this pull request. |
|
Hi @ScottMansfield , is this PR still in draft? If not can you please fill in the details for the PR. |
These updates are derived from @anmolg1997 PR on the community repo: google/adk-python-community#104
|
I believe this is ready for review but I'm testing it with a real firestore instance |
tests/unittests/integrations/firestore/test_firestore_session_service.py
Show resolved
Hide resolved
src/google/adk/integrations/firestore/firestore_memory_service.py
Outdated
Show resolved
Hide resolved
src/google/adk/integrations/firestore/firestore_session_service.py
Outdated
Show resolved
Hide resolved
src/google/adk/integrations/firestore/firestore_session_service.py
Outdated
Show resolved
Hide resolved
src/google/adk/integrations/firestore/firestore_session_service.py
Outdated
Show resolved
Hide resolved
src/google/adk/integrations/firestore/firestore_session_service.py
Outdated
Show resolved
Hide resolved
END_PUBLIC Merge #5088 Adding support for Firestore for both session and memory storage. This started by copying the Firestore support from the Java ADK into the Python ADK and also takes inspiration from @anmolg1997's PR google/adk-python-community#104. It does things differently from both. Firestore contains a hierarchical set of data for sessions: ``` Hierarchy for sessions: adk-session ↳ <app name> ↳ users ↳ <user ID> ↳ sessions ↳ <session ID> ↳ events ↳ <event ID> Hierarchy for shared App/User state configurations: app_states ↳ <app name> user_states ↳ <app name> ↳ users ↳ <user ID> Hierarchy for memory: memories ↳ <memory ID> ``` The firestore memory service creates a top-level collection that hold indexed memories when sessions are added. ### Link to Issue or Description of Change This is from an existing customer request for firestore support. ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. ``` $ pytest tests/unittests/integrations/firestore/ ===================================== test session starts ====================================== platform darwin -- Python 3.11.14, pytest-9.0.2, pluggy-1.6.0 rootdir: /Users/scottmansfield/projects/adk-python configfile: pyproject.toml plugins: mock-3.15.1, xdist-3.8.0, langsmith-0.7.23, asyncio-1.3.0, anyio-4.13.0 asyncio: mode=Mode.AUTO, debug=False, asyncio_default_fixture_loop_scope=function, asyncio_default_test_loop_scope=function collected 29 items tests/unittests/integrations/firestore/test_firestore_memory_service.py ............. [ 44%] tests/unittests/integrations/firestore/test_firestore_session_service.py ................ [100%] ======================================= warnings summary ======================================= src/google/adk/features/_feature_decorator.py:72 /Users/scottmansfield/projects/adk-python/src/google/adk/features/_feature_decorator.py:72: UserWarning: [EXPERIMENTAL] feature FeatureName.PLUGGABLE_AUTH is enabled. check_feature_enabled() -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html ================================ 29 passed, 1 warning in 1.44s ================================= ``` **Manual End-to-End (E2E) Tests:** I created a demo app locally to test the session and memory storage with a real firebase instance. It successfully records sessions and memories, verified by manually checking the cloud console. Memory did require an index, which will be created by the user the first time the memory session is used. Firestore has a specific deep link that it will create to give the exact index needed. After that, memory worked fine. ### 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. COPYBARA_INTEGRATE_REVIEW=#5088 from ScottMansfield:feat/firestore 99efca5 PiperOrigin-RevId: 899328760
|
Thank you @ScottMansfield for your contribution! 🎉 Your changes have been successfully imported and merged via Copybara in commit 1a9df8f. Closing this PR as the changes are now in the main branch. |
Adding support for Firestore for both session and memory storage.
This started by copying the Firestore support from the Java ADK into the Python ADK and also takes inspiration from @anmolg1997's PR google/adk-python-community#104. It does things differently from both.
Firestore contains a hierarchical set of data for sessions:
The firestore memory service creates a top-level collection that hold indexed memories when sessions are added.
Link to Issue or Description of Change
This is from an existing customer request for firestore support.
Testing Plan
Unit Tests:
Manual End-to-End (E2E) Tests:
I created a demo app locally to test the session and memory storage with a real firebase instance. It successfully records sessions and memories, verified by manually checking the cloud console.
Memory did require an index, which will be created by the user the first time the memory session is used. Firestore has a specific deep link that it will create to give the exact index needed. After that, memory worked fine.
Checklist