-
Notifications
You must be signed in to change notification settings - Fork 2.6k
docs: Add PostgreSQL session storage sample and documentation #3926
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
Add a comprehensive sample demonstrating how to use DatabaseSessionService with PostgreSQL, including: - README with schema documentation and configuration guide - Sample agent and main.py for session persistence demo - Docker Compose file for local PostgreSQL setup Related to google#3916
Summary of ChangesHello @hiroakis, 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 addresses a gap in user guidance by introducing a comprehensive sample and documentation for integrating PostgreSQL with the 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 adds a valuable sample and documentation for using PostgreSQL as a session store with DatabaseSessionService. The code and documentation are well-structured and clear. I have a few suggestions to improve the sample code's robustness and the documentation's clarity. Specifically, I've recommended using timezone-aware datetimes in the sample agent and improving the consistency and clarity of instructions in the README.md file.
| Install the required async PostgreSQL driver: | ||
|
|
||
| ```bash | ||
| pip install asyncpg | ||
| ``` |
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.
The installation instructions are split between the 'Installation' section and 'Running the Sample' section (step 3). This can be confusing. It would be clearer to provide the complete installation command in this 'Installation' section. With this change, step 3 under 'Running the Sample' (lines 155-159) becomes redundant and can be removed for better clarity.
| Install the required async PostgreSQL driver: | |
| ```bash | |
| pip install asyncpg | |
| ``` | |
| Install the required Python packages: | |
| ```bash | |
| pip install google-adk asyncpg greenlet |
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.
Done
|
|
||
| ```bash | ||
| POSTGRES_URL=postgresql+asyncpg://postgres:postgres@localhost:5432/adk_sessions | ||
| GOOGLE_CLOUD_PROJECT={{ your project }} |
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.
The placeholder {{ your project }} is a bit unconventional. Using a more standard placeholder format like <your-gcp-project-id> would be clearer for users and consistent with common documentation practices. It also makes it more obvious that this is a value they need to replace.
| GOOGLE_CLOUD_PROJECT={{ your project }} | |
| GOOGLE_CLOUD_PROJECT=<your-gcp-project-id> |
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.
Done
| from datetime import datetime | ||
|
|
||
| from google.adk.agents.llm_agent import Agent | ||
|
|
||
|
|
||
| def get_current_time() -> str: | ||
| """Get the current time. | ||
|
|
||
| Returns: | ||
| A string with the current time in ISO format. | ||
| """ | ||
| return datetime.now().strftime("%Y-%m-%d %H:%M:%S") |
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.
The current implementation of get_current_time uses datetime.now(), which produces a timezone-naive datetime object. This can lead to ambiguity and bugs when dealing with different timezones. It's best practice to use timezone-aware datetimes, for instance by using UTC.
Additionally, the docstring claims to return an ISO format string, but the strftime format "%Y-%m-%d %H:%M:%S" is not the standard ISO 8601 format. Using datetime.isoformat() on a timezone-aware datetime object is the correct way to produce an ISO 8601 compliant string. This change also requires importing timezone from datetime.
| from datetime import datetime | |
| from google.adk.agents.llm_agent import Agent | |
| def get_current_time() -> str: | |
| """Get the current time. | |
| Returns: | |
| A string with the current time in ISO format. | |
| """ | |
| return datetime.now().strftime("%Y-%m-%d %H:%M:%S") | |
| from datetime import datetime, timezone | |
| from google.adk.agents.llm_agent import Agent | |
| def get_current_time() -> str: | |
| """Get the current time. | |
| Returns: | |
| A string with the current time in ISO 8601 format. | |
| """ | |
| return datetime.now(timezone.utc).isoformat() |
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.
Done
- Consolidate installation instructions in Installation section - Remove duplicate step 3 and renumber steps - Use standard placeholder format (<your-gcp-project-id>) - Use timezone-aware datetime with ISO 8601 format
|
I've addressed all the feedback in 66bc944
|
|
Great addition, @hiroakis. I will make the corresponding change in the service documentation to correct the SQLite example: google/adk-docs#1075 |
|
Hi @hiroakis , Thank you for your contribution! We appreciate you taking the time to submit this pull request. |
|
@ryanaiagent Fixed in 30bbad2 ! Note: Running autoformat.sh also fixed contributing/samples/gepa/run_experiment.py and contributing/samples/gepa/experiment.py, so I've included those changes in the commit as well. |
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
Problem:
While
DatabaseSessionServicealready supports PostgreSQL through SQLAlchemy, there is no documentation or sample code showing users how to configure and use it.Solution:
Add a comprehensive sample under
contributing/samples/postgres_session_service/that demonstrates:DatabaseSessionServicewith PostgreSQLTesting Plan
Unit Tests:
This is a documentation-only change (new sample), so no new unit tests are required. Existing tests continue to pass.
Manual End-to-End (E2E) Tests:
Tested locally with the following steps:
docker compose up -dpip install google-adk asyncpg greenletto install the required packagespython main.py- session created successfullypython main.pyagain - previous session resumed with event historyChecklist
Additional context
This PR adds documentation and a working sample for an already-supported feature. The DatabaseSessionService class already handles PostgreSQL through its DynamicJSON type decorator which uses JSONB for PostgreSQL.
Files added: