-
Notifications
You must be signed in to change notification settings - Fork 8
feat: making sure agent call invocation on runtime is asyncio friendly and runs in a serial manner #205
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: develop
Are you sure you want to change the base?
Conversation
…y and runs in a serial manner Signed-off-by: Tharindu Dissanayake <[email protected]>
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.
Pull request overview
This PR refactors the Session class to make agent invocations asyncio-friendly with serial execution guarantees per session through async context managers and locks.
Changes:
- Introduced async context manager support (
__aenter__/__aexit__) forSessionwith an asyncio lock to ensure serial execution per session - Replaced session cache key constants with
Session.Keysenumeration - Added new
Session.current()andSession.get_all()methods while deprecating older APIs (get_current_session_id(),get_all_keys(),set_context(),reset_context()) - Updated storage implementations (Redis, DynamoDB) to use the new
get_all(volatile=False)API - Modified
Runtime.run()andAuxiliaryCachemethods to use the new session context management
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| ak-py/src/agentkernel/core/base.py | Core refactoring: Added async context manager, Keys enum, new session methods, deprecated old APIs |
| ak-py/src/agentkernel/core/runtime.py | Updated to use async with session pattern and new Session.current() API |
| ak-py/src/agentkernel/core/service.py | Bug: Uses synchronous with instead of async with for session |
| ak-py/src/agentkernel/core/session/redis.py | Updated store() to use new get_all(volatile=False) API |
| ak-py/src/agentkernel/core/session/dynamodb.py | Updated store() to use new get_all(volatile=False) API |
| ak-py/src/agentkernel/framework/openai/openai.py | Removed SessionABC inheritance from OpenAISession |
| ak-py/tests/test_session.py | Added tests for new APIs, repr, get_all filtering; contains identity comparison bugs |
| ak-py/tests/test_runtime.py | Updated tests to use new async with pattern and error messages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ved from Session Signed-off-by: Tharindu Dissanayake <[email protected]>
Signed-off-by: Tharindu Dissanayake <[email protected]>
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… serial access verification Signed-off-by: Tharindu Dissanayake <[email protected]>
Signed-off-by: Tharindu Dissanayake <[email protected]>
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Tharindu Dissanayake <[email protected]>
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Tharindu Dissanayake <[email protected]>
Description
This PR make the following changes to the
Sessionimplementation.Runtimeisasynciofriendly and makes sure invocations run in coroutines concurrently while making sure invocations for the same session runs seriallySessionimplementationSessionclass to 100%Type of Change
Changes Made
Sessionhave been replaced with the enumerationSession.KeysSession.get_current_session_id()has been replaced withSession.current()that returns the current session directlySessionnow includes a properreprSessioncan now support entry/exit semantics forasync withthat mark it as the current session for the coroutine and also acquire an async lock to force serial execution of coroutines related to the same session.Session.get_all_keys()has been replaced by the producerSession.get_all()that support filtering of session data objects by durability and volatility making it easier for storage implementations as they do not have to check session data object keys and filter accordingly.Session.set_context()andSession.reset_context()has been deprecated in favor ofasync withentry/exit semantics.AgentService.run_multi()has been modified to just callRuntime.run()without session locking. Volatile cache clearing and storage updating has also been removed. These steps not execute withinRuntime.run().Testing
Checklist