-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Implement Typesense-based session service #3129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @amedeorizzo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's session management capabilities by integrating Typesense as a new, highly performant session service. This provides an alternative to the existing database-backed solution, offering flexibility in choosing the underlying storage for session data and events. The change also includes a refactoring of common state manipulation utilities, making them reusable across different session service implementations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new TypesenseSessionService for session management and refactors some shared state logic into a utility module. The implementation is quite thorough. My review focuses on a critical security vulnerability related to the use of pickle, a high-severity bug in session listing that could lead to data truncation, and several medium-severity suggestions to improve code consistency, clarity, and adherence to best practices.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Response from ADK Triaging Agent Hello @amedeorizzo, thank you for your contribution! To help us review this PR, could you please associate it with a GitHub issue? If one doesn't exist, please create one. Also, could you please add a This information is very helpful for our review process. Thanks! |
|
Response from ADK Triaging Agent Hello @amedeorizzo, thank you for creating this PR and for adding the detailed testing plan! I've noticed a couple of things that need to be addressed before we can proceed with the review:
Addressing these items will help us move forward with the review process. Thanks! |
…ession service; remove duplicate _merge_state
Closes: #3226
Introduce a new session service utilizing Typesense for state management and session handling.
Testing Plan
I've written 14 comprehensive unit tests in
tests/unittests/sessions/test_typesense_session_service.pythat cover all the functionality of TypesenseSessionService. The tests verify initialization with different protocols (HTTP and HTTPS), all CRUD operations for sessions and events, state management with the different prefixes (app:, user:, and session-level), pagination for large datasets, and proper error handling.You can run the tests with:
pytest tests/unittests/sessions/test_typesense_session_service.py -vIntegration Testing
I've tested the service against a real Typesense server running locally in Docker. This involved creating sessions with complex nested state, adding events with various metadata, listing sessions for users with large numbers of sessions (to verify pagination works), and verifying that all data persists correctly.
I also tested against Typesense Cloud using HTTPS to make sure it works in production cloud environments, not just locally. The service connected successfully and all operations worked as expected.
For large datasets, I created 1000 sessions with 10 events each and verified that pagination works correctly in list_sessions.
Migration Script Testing
I tested the migration script by setting up a PostgreSQL database with sample data created using DatabaseSessionService, then running the migration script in dry-run mode first to verify it detects the correct number of records, and finally performing the actual migration. After migration, I verified data integrity by checking document counts in each Typesense collection and spot-checking that the data was transformed correctly.
Production Testing
I tested the implementation in a real-world scenario using my private implementation of adk-python. This involved building a Docker image that bundles the modified ADK code, running the agent in a container connected to Typesense, and verifying that sessions persist correctly across container restarts and multi-turn conversations work properly.
The Docker build process works correctly with the local ADK changes, and the agent successfully initializes
TypesenseSessionService and uses it for all session storage operations.
Backward Compatibility
I verified that when the typesense package is not installed, ADK still imports successfully and the other session services
(DatabaseSessionService and InMemorySessionService) continue to work normally.