- Notifications
You must be signed in to change notification settings - Fork2.8k
Add session metadata support to SQLAlchemySession#1939
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 ourterms of service andprivacy 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
Adds the ability to store and query metadata for sessions using a newagent_sessions_metadata table. This allows applications to associatemetadata like owner_id, title, or tags with sessions.New methods:- set_metadata() - store metadata key-value pairs- get_metadata() - retrieve metadata for a session- delete_metadata() - remove metadata- find_sessions_by_metadata() - find sessions by metadata valuesAlso updates clear_session() to clean up metadata when clearing a session.Fixesopenai#1938
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When yousign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # Use dialect-specific UPSERT | ||
| stmt=stmt.on_conflict_do_update( | ||
| index_elements=["session_id","key"], | ||
| set_={ | ||
| "value":stmt.excluded.value, | ||
| "updated_at":sql_text("CURRENT_TIMESTAMP"), | ||
| }, |
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.
Use MySQL-compatible UPSERT in set_metadata
The new metadata UPSERT unconditionally callsinsert(...).on_conflict_do_update(...) after importing the MySQL dialect’sinsert. The MySQL insert construct doesn’t implementon_conflict_do_update; it only exposeson_duplicate_key_update. As a result,set_metadata() raisesAttributeError for any MySQL-backed session and metadata cannot be written. Consider branching toon_duplicate_key_update for MySQL instead of calling the PostgreSQL/SQLite API.
Useful? React with 👍 / 👎.
Thanks for sending this code change. It may work but we still hesitate having this change as it could be a specific change for your use case (see#1938 (comment) for more details on what I think). So, for now, please inherit the built-in class and add these functionalities on your own sub class. |
MySQL uses on_duplicate_key_update() instead of on_conflict_do_update().This fixes AttributeError when using set_metadata() with MySQL databases.
This PR is stale because it has been open for 10 days with no activity. |
Adds the ability to store and query metadata for sessions using a new agent_sessions_metadata table. This allows applications to associate metadata like owner_id, title, or tags with sessions.
New methods:
Also updates clear_session() to clean up metadata when clearing a session.
Feat:#1938