-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(cli): validate agent folder names for agent_engine #4246
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?
fix(cli): validate agent folder names for agent_engine #4246
Conversation
Add early validation for agent folder names and temp_folder to ensure they are valid Python identifiers before deployment. This surfaces clear, actionable errors instead of cryptic syntax failures when folder names include characters like dashes. Fixes google#2902
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @shaikhmudassir, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Response from ADK Triaging Agent Hello @shaikhmudassir, thank you for creating this PR! Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). You can visit https://cla.developers.google.com/ to see your current agreements or to sign a new one. This information will help reviewers to review your PR more efficiently. Thanks! |
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.
Code Review
This pull request introduces important validation for agent folder names and temporary folder names to ensure they are valid Python identifiers. This is a great improvement as it prevents cryptic downstream errors and provides clear, actionable feedback to the user. My main suggestion is to use click.ClickException for error handling instead of printing to stderr and returning. This aligns better with CLI best practices by ensuring the command exits with a non-zero status code on failure, which is important for scripting and automation.
Replace click.secho + return with click.ClickException for better CLI error handling and shell scripting compatibility.
Added early validation for agent folder names and temp_folder to ensure they are valid Python identifiers before deployment. This surfaces clear, actionable errors instead of cryptic syntax failures when folder names include characters
like dashes.
Fixes #2902
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):
2. Or, if no issue exists, describe the change:
If applicable, please follow the issue templates to provide as much detail as
possible.
Problem:
adk deploy agent_enginefails with a cryptic "invalid syntax" error from Agent Engine, when the agent folder name contains characters that are invalid in Python module names (e.g., dashes).Solution:
Add early validation in
to_agent_engineto ensure the agent folder name and the optional temp_folder are valid Python identifiers (using isidentifier()). If they are invalid, the CLI exits early with a clear error message, instead of letting Agent Engine fail later with an unreadable syntax error.Testing Plan
Please describe the tests that you ran to verify your changes. This is required
for all PRs that are not small documentation or typo fixes.
Unit Tests:
I ran
pytest ./tests/unittestsand 3983 passed, 2168 warnings in 53.86sManual End-to-End (E2E) Tests:
Invalid agent folder name (with dash):
adk deploy agent_engine --api_key=MY_API_KEY agent-folder-name
Observed:
Error: Agent folder name "agent-folder-name" must be a valid identifier consisting of letters, digits, and underscores.
from agent-folder-name.agent import root_agentValid agent folder name:
adk deploy agent_engine --api_key=MY_API_KEY agent_folder_name
Observed:
Checklist
Additional context
Add any other context or screenshots about the feature request here.