-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add .yml file extension support for agent file watcher #3860
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
|
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 @yamadashy, 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 agent file watcher to recognize Highlights
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
|
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 adds support for the .yml file extension to the agent file watcher for hot reloading. While the change is a good first step, I've suggested a small refactoring for better readability. More critically, the agent loading logic in AgentLoader doesn't seem to support .yml files, which will likely cause reloads to fail. This needs to be addressed to make the feature work as intended. Please see my detailed comments for specifics.
|
|
||
| def on_modified(self, event): | ||
| if not (event.src_path.endswith(".py") or event.src_path.endswith(".yaml")): | ||
| if not (event.src_path.endswith(".py") or event.src_path.endswith(".yaml") or event.src_path.endswith(".yml")): |
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.
While this change correctly adds .yml to the file watcher for hot reloading, the underlying AgentLoader in src/google/adk/cli/utils/agent_loader.py does not appear to support loading agents from .yml files. Functions like _load_from_yaml_config and _determine_agent_language specifically look for root_agent.yaml. This discrepancy will likely cause reloads triggered by .yml file changes to fail. To fully implement this feature, the agent loading logic should also be updated to recognize and load .yml configuration files.
|
|
||
| def on_modified(self, event): | ||
| if not (event.src_path.endswith(".py") or event.src_path.endswith(".yaml")): | ||
| if not (event.src_path.endswith(".py") or event.src_path.endswith(".yaml") or event.src_path.endswith(".yml")): |
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 chain of or conditions can be made more concise and maintainable. The endswith() string method can accept a tuple of suffixes to check against, which is a more Pythonic way to handle this.
| if not (event.src_path.endswith(".py") or event.src_path.endswith(".yaml") or event.src_path.endswith(".yml")): | |
| if not event.src_path.endswith(('.py', '.yaml', '.yml')): |
Add .yml extension to the watched file extensions in AgentChangeEventHandler.on_modified() alongside .py and .yaml files. Fixes google#3834
2098634 to
493e59c
Compare
|
Response from ADK Triaging Agent Hello @yamadashy, thank you for your contribution! To help reviewers evaluate your changes, could you please add a Additionally, it appears the CLA (Contributor License Agreement) check has failed. Please ensure you have signed the CLA, as we cannot merge this pull request until that is complete. Thank you! |
667dd80 to
493e59c
Compare
Summary
Add
.ymlextension to the watched file extensions inAgentChangeEventHandler.on_modified()alongside.pyand.yamlfiles.This allows the
--reload_agentsflag to properly detect changes in.ymlconfiguration files.Fixes #3834
Scope
This PR only adds
.ymlto the file watcher for hot reload. It does not add support forroot_agent.ymlas an agent definition file (that would require changes toAgentLoaderand could be addressed in a separate PR if needed).Testing Plan
adk web --reload_agents, modify a.ymlfile in the agent directory, and verify the reload is triggered