feat: add ADK agent samples for Parameter Manager#14103
feat: add ADK agent samples for Parameter Manager#14103amitmodak wants to merge 1 commit intoGoogleCloudPlatform:mainfrom
Conversation
product-auto-label
Bot
added
the
samples
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new samples and tests for using the Agent Development Kit (ADK) with Google Cloud Parameter Manager, covering both global and regional configurations. The review feedback highlights several areas for improvement, including correcting the regional API endpoint format and model version names, resolving environment variable mismatches in the documentation, and ensuring that fetched payloads are actually utilized in the sample code. Additionally, the Nox configuration should be updated to include modern Python versions in the test suite to ensure proper coverage.
|
|
||
| @pytest.fixture() | ||
| def client(location: str) -> parametermanager_v1.ParameterManagerClient: | ||
| api_endpoint = f"parametermanager.{location}.rep.googleapis.com" |
There was a problem hiding this comment.
The API endpoint format parametermanager.{location}.rep.googleapis.com seems incorrect for a public sample. The standard regional endpoint for Parameter Manager is typically LOCATION-parametermanager.googleapis.com.
| api_endpoint = f"parametermanager.{location}.rep.googleapis.com" | |
| api_endpoint = f"{location}-parametermanager.googleapis.com" |
| ```env | ||
| GOOGLE_GENAI_USE_VERTEXAI=1 | ||
| GOOGLE_CLOUD_PROJECT=your-project-id | ||
| GOOGLE_CLOUD_LOCATION=us-central1 |
There was a problem hiding this comment.
The environment variable GOOGLE_CLOUD_LOCATION is not used by the global agent sample. Consider removing this line to avoid confusion for users setting up the global agent.
| ```env | ||
| GOOGLE_GENAI_USE_VERTEXAI=1 | ||
| GOOGLE_CLOUD_PROJECT=your-project-id | ||
| GOOGLE_CLOUD_LOCATION=us-central1 |
There was a problem hiding this comment.
The regional agent code in agent_regional/agent.py expects the environment variable GOOGLE_CLOUD_PROJECT_LOCATION, but the README uses GOOGLE_CLOUD_LOCATION. Please update this to match the implementation.
| GOOGLE_CLOUD_LOCATION=us-central1 | |
| GOOGLE_CLOUD_PROJECT_LOCATION=us-central1 |
| parameter_payload = client.get_parameter(resource_name) | ||
| print("Successfully fetched parameter.") |
There was a problem hiding this comment.
The variable parameter_payload is assigned but never used. In a sample, it is helpful to demonstrate how to access the fetched data, for example by printing it.
| parameter_payload = client.get_parameter(resource_name) | |
| print("Successfully fetched parameter.") | |
| parameter_payload = client.get_parameter(resource_name) | |
| print(f"Successfully fetched parameter: {parameter_payload}") |
|
|
||
| # Initialize Agent | ||
| root_agent = Agent( | ||
| model='gemini-2.5-flash', |
There was a problem hiding this comment.
The model name gemini-2.5-flash appears to be a typo. Standard Gemini model versions are typically 1.5 or 2.0. Please verify the intended model version.
| model='gemini-2.5-flash', | |
| model='gemini-1.5-flash', |
|
|
||
| TEST_CONFIG_OVERRIDE = { | ||
| # You can opt out from the test for specific Python versions. | ||
| "ignored_versions": ["2.7", "3.7", "3.8", "3.9", "3.10", "3.11", "3.12"], |
There was a problem hiding this comment.
The ignored_versions list includes all common Python 3 versions (3.9 through 3.12). This will cause the tests to be skipped in most CI environments. Consider removing modern versions from this list to ensure the samples are properly tested.
| "ignored_versions": ["2.7", "3.7", "3.8", "3.9", "3.10", "3.11", "3.12"], | |
| "ignored_versions": ["2.7", "3.7", "3.8"], |
| parameter_payload = client.get_parameter(resource_name) | ||
| print("Successfully fetched parameter.") |
There was a problem hiding this comment.
The variable parameter_payload is assigned but never used. In a sample, it is helpful to demonstrate how to access the fetched data, for example by printing it.
| parameter_payload = client.get_parameter(resource_name) | |
| print("Successfully fetched parameter.") | |
| parameter_payload = client.get_parameter(resource_name) | |
| print(f"Successfully fetched parameter: {parameter_payload}") |
|
|
||
| # Initialize Agent | ||
| root_agent = Agent( | ||
| model='gemini-2.5-flash', |
There was a problem hiding this comment.
The model name gemini-2.5-flash appears to be a typo. Standard Gemini model versions are typically 1.5 or 2.0. Please verify the intended model version.
| model='gemini-2.5-flash', | |
| model='gemini-1.5-flash', |
|
|
||
| TEST_CONFIG_OVERRIDE = { | ||
| # You can opt out from the test for specific Python versions. | ||
| "ignored_versions": ["2.7", "3.7", "3.8", "3.9", "3.10", "3.11", "3.12"], |
There was a problem hiding this comment.
The ignored_versions list includes all common Python 3 versions (3.9 through 3.12). This will cause the tests to be skipped in most CI environments. Consider removing modern versions from this list to ensure the samples are properly tested.
| "ignored_versions": ["2.7", "3.7", "3.8", "3.9", "3.10", "3.11", "3.12"], | |
| "ignored_versions": ["2.7", "3.7", "3.8"], |
| version_path = f"projects/{project_id}/locations/{location}/parameters/{parameter_id}/versions/1" | ||
| parameter_path = f"projects/{project_id}/locations/{location}/parameters/{parameter_id}" |
There was a problem hiding this comment.
It is recommended to use the client's helper methods (e.g., parameter_version_path and parameter_path) to construct resource names instead of hardcoded strings. This ensures consistency and reduces the risk of formatting errors.
| version_path = f"projects/{project_id}/locations/{location}/parameters/{parameter_id}/versions/1" | |
| parameter_path = f"projects/{project_id}/locations/{location}/parameters/{parameter_id}" | |
| version_path = client.parameter_version_path(project_id, location, parameter_id, "1") | |
| parameter_path = client.parameter_path(project_id, location, parameter_id) |
1 participant
Description
Fixes #
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
nox -s py-3.9(see Test Environment Setup)nox -s lint(see Test Environment Setup)