-
Notifications
You must be signed in to change notification settings - Fork 314
feat(byo): add lifespan hook for BYO agents #1064
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
I'm looking for a way to be able to run background code in a BYO agent. Whilst this is possible by setting up your own event loop, it would be nice if we could hook into the existing [FastAPI lifespan](https://fastapi.tiangolo.com/advanced/events/#lifespan) instead. So I figured I'd propose doing just that... That being said, I'm completely open to alternatives if anyone knows a better way of doing this 😄 Signed-off-by: Brian Fox <[email protected]>
f5d469f to
1febeb3
Compare
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 adds support for optional user-defined lifespan context managers in the KAgent ADK framework. Users can now define a lifespan function in their agent module that will be executed during the FastAPI application lifecycle, enabling custom setup and teardown logic.
Key Changes:
- Added a sample
lifespan.pymodule demonstrating the lifespan pattern with setup and teardown logging - Extended
KAgentAppto accept an optionallifespanparameter and compose it with the built-in token service lifespan - Modified the CLI to dynamically import and use user-defined lifespan functions from agent modules
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| python/samples/adk/basic/basic/lifespan.py | Sample implementation of user-defined lifespan with logging |
| python/samples/adk/basic/basic/init.py | Exports the new lifespan function for discoverability |
| python/packages/kagent-adk/src/kagent/adk/cli.py | Dynamically imports lifespan from agent modules and passes it to KAgentApp |
| python/packages/kagent-adk/src/kagent/adk/_a2a.py | Adds lifespan parameter and composition logic to integrate user-defined and built-in lifespans |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| kagent_app = KAgentApp(root_agent, agent_card, app_cfg.url, app_cfg.app_name) | ||
| # Attempt to import optional user-defined lifespan(app) from the agent package | ||
| lifespan = None | ||
| try: |
Copilot
AI
Nov 4, 2025
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 dynamic import uses the agent name directly without adjusting sys.path to include working_dir. If the agent module is in a subdirectory (e.g., working_dir/name), this import may fail. Consider using sys.path.insert(0, working_dir) before the import to ensure the module can be found, similar to how AgentLoader handles the working directory.
| try: | |
| try: | |
| sys.path.insert(0, working_dir) |
| if hasattr(module_candidate, "lifespan"): | ||
| lifespan = module_candidate.lifespan | ||
| except Exception: | ||
| logger.exception("Failed to load agent module for lifespan") |
Copilot
AI
Nov 4, 2025
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 error message 'Failed to load agent module for lifespan' is too generic and doesn't indicate which module failed to load. Consider including the module name in the error message: f\"Failed to load agent module '{name}' for lifespan\" to aid in debugging.
| logger.exception("Failed to load agent module for lifespan") | |
| logger.exception(f"Failed to load agent module '{name}' for lifespan") |
| if self._lifespan is not None: | ||
| app = FastAPI(lifespan=self._lifespan) | ||
| else: | ||
| app = FastAPI() |
Copilot
AI
Nov 4, 2025
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 build_local() method doesn't compose the user-defined lifespan with any base lifespan like build() does with token_service.lifespan(). While this may be intentional for local mode, it creates inconsistency. If users define a lifespan expecting composition behavior (e.g., wrapping around a base lifespan), it won't work in local mode. Consider either using _compose_lifespan() with a no-op base lifespan or documenting this behavioral difference.
| if self._lifespan is not None: | |
| app = FastAPI(lifespan=self._lifespan) | |
| else: | |
| app = FastAPI() | |
| from contextlib import asynccontextmanager | |
| @asynccontextmanager | |
| async def noop_lifespan(app): | |
| yield | |
| composed_lifespan = self._compose_lifespan(noop_lifespan) | |
| app = FastAPI(lifespan=composed_lifespan) |
| @@ -1,4 +1,6 @@ | |||
| import asyncio | |||
| import importlib | |||
| import sys | |||
Copilot
AI
Nov 4, 2025
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.
Import of 'sys' is not used.
| import sys |
I'm looking for a way to be able to run background process(es) in a BYO agent. Whilst this is possible by setting up your own event loop, I thought it might be nicer(?) if we could hook into the existing FastAPI lifespan instead. So I figured I'd propose doing just that... That being said, I'm completely open to alternatives if anyone knows a better way of doing this 😄