Skip to content

feat(cli): add --lifespan option to api_server command#5938

Open
kemurayama wants to merge 1 commit into
google:v2from
kemurayama:feat/api-server-lifespan
Open

feat(cli): add --lifespan option to api_server command#5938
kemurayama wants to merge 1 commit into
google:v2from
kemurayama:feat/api-server-lifespan

Conversation

@kemurayama
Copy link
Copy Markdown

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

(Note: If there is no specific issue, please leave the above blank or remove them, and fill in section 2 below.)

2. Or, if no issue exists, describe the change:

Problem:
Currently, there is no way to register a lifespan handler (startup and shutdown logic) directly from the adk api_server CLI command in v1 and v2. While the underlying get_fast_api_app function accepts a lifespan argument, the CLI command does not expose it, preventing users from running custom setup or teardown logic for their agents.

Solution:
This PR introduces a --lifespan option to the adk api_server CLI command. This allows users to specify a lifespan context manager dynamically via its import path (e.g., path.to.module.lifespan_handler).

  • Added --lifespan option to the api_server command definition.
  • Implemented _load_lifespan_handler helper to dynamically import the lifespan handler using its string module path.
  • Updated cli_api_server to dynamically load the lifespan handler and pass it down to get_fast_api_app.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Summary of passed pytest results:
Added test_cli_api_server_passes_lifespan in tests/unittests/cli/utils/test_cli_tools_click.py to verify that the CLI correctly parses --lifespan, loads the module, and passes the handler to get_fast_api_app.

Command:

pytest tests/unittests/cli/utils/test_cli_tools_click.py -k test_cli_api_server_passes_lifespan

@adk-bot adk-bot added the core [Component] This issue is related to the core interface and implementation label Jun 2, 2026
@rohityan rohityan self-assigned this Jun 5, 2026
@rohityan
Copy link
Copy Markdown
Collaborator

rohityan commented Jun 5, 2026

/adk-pr-analyze

@adk-bot
Copy link
Copy Markdown
Collaborator

adk-bot commented Jun 5, 2026

I have successfully retrieved, checked, and analyzed Pull Request #5938 in a strictly read-only manner.

The complete, detailed review has been generated and stored as an artifact:
👉 analysis_results.md


🔍 Key Findings & Triage Summary

Item Status Root Cause & Analysis
Google CLA Check SUCCESS The mandatory cla/google status check is signed and verified on GitHub.
Aesthetic & Value VALUABLE Resolves a major usability gap (see related issue #1683) by allowing users to declare dynamic startup/shutdown lifespans natively from the api_server CLI.
Code & Style Checks FAILED A NameError: name 'Any' is not defined occurs because Any is used in the signature of the new helper function _load_lifespan_handler in cli_tools_click.py but is never imported from typing.
Unit Test Suites FAILED The mock in the newly added test_cli_api_server_passes_lifespan in test_cli_tools_click.py targets cli_tools_click.get_fast_api_app. Since that function is imported dynamically inside the execution body from .fast_api, the mock is completely bypassed, causing the real application logic to attempt to execute. At the same time, the test unit contains a docstring that violates the strict 80-character line limit under pyink.

Recommendation

Push Back (Request Changes): The core feature architecture is stellar and very useful. However, the contributor must:

  1. Import Any from typing in cli_tools_click.py.
  2. Correct the unit test monkeypatch target to "google.adk.cli.fast_api.get_fast_api_app" in test_cli_tools_click.py.
  3. Shorten the test docstring to respect the strict 80-character formatting limit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core [Component] This issue is related to the core interface and implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants