-
Notifications
You must be signed in to change notification settings - Fork 354
feat(all): add support for ADK #671
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
Conversation
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
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 ADK (Agent Development Kit) to kagent, representing a significant architectural shift from the previous kagent-engine approach to individual agent deployments running ADK. Key changes include removal of the centralized engine, introduction of ADK integration with new Python packages, and updates to the Kubernetes deployment configuration.
- Complete removal of
kagent-engineand related autogen integration - Introduction of new
kagent-adkPython package for ADK agent integration - Migration from Python 3.12 to 3.13 and restructured workspace configuration
Reviewed Changes
Copilot reviewed 148 out of 219 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| python/src/autogenstudio/ | Removed entire autogenstudio module including web managers, validation, and evaluation components |
| python/packages/kagent-adk/ | New ADK integration package with agent configuration models and A2A protocol support |
| python/packages/kagent/ | Restructured kagent package with simplified CLI for static agent deployment |
| python/pyproject.toml | Converted to uv workspace configuration with updated dependencies |
| helm/kagent/ | Updated Helm chart removing engine deployment and adding grafana-mcp sidecar |
| go/internal/ | Removed autogen client integration and simplified HTTP server handlers |
Comments suppressed due to low confidence (1)
python/packages/kagent-adk/src/kagent_adk/_session_service.py:17
- [nitpick] The class name
KAgentSessionServiceis inconsistent with the filename which uses lowercase 'kagent'. Consider renaming toKagentSessionServiceto match the original naming convention or update the filename.
class KAgentSessionService(BaseSessionService):
| elif self.model.type == "azure_openai": | ||
| model = LiteLlm(model=f"azure/{self.model.model}") | ||
| elif self.model.type == "gemini": | ||
| model = self.model.model |
Copilot
AI
Jul 31, 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 Gemini model type handling is incomplete. When self.model.type == "gemini", the code assigns model = self.model.model which appears to be a string, but the Agent constructor expects a model instance like the other branches. This will likely cause a runtime error.
| model = self.model.model | |
| model = GeminiLLM(model=self.model.model) |
| HTTPXClientInstrumentor().instrument() | ||
| OpenAIInstrumentor().instrument() | ||
|
|
||
| with open(filepath, "r") as f: |
Copilot
AI
Jul 31, 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 file is opened in text mode without specifying encoding. Consider adding encoding='utf-8' to ensure consistent behavior across different platforms and avoid potential encoding issues.
| with open(filepath, "r") as f: | |
| with open(filepath, "r", encoding="utf-8") as f: |
| - name: grafana-mcp # Temporary workaround until kmcp is ready. | ||
| securityContext: | ||
| {{- toYaml .Values.securityContext | nindent 12 }} | ||
| image: mcp/grafana:latest |
Copilot
AI
Jul 31, 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.
Using the latest tag for the mcp/grafana image in production is a security and stability risk. The image should be pinned to a specific version to ensure reproducible deployments and avoid unexpected changes.
| image: mcp/grafana:latest | |
| image: mcp/grafana:8.5.0 |
| raise TypeError(f"Callable must return a Runner instance, got {type(resolved_runner)}") | ||
|
|
||
| # Cache the resolved runner for future calls | ||
| # self._runner = resolved_runner |
Copilot
AI
Jul 31, 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.
[nitpick] The commented out caching logic # self._runner = resolved_runner suggests intended functionality that was disabled. Consider either implementing proper caching or removing the comment to avoid confusion about the intended behavior.
| # self._runner = resolved_runner | |
| self._cached_runner = resolved_runner |
| - name: GRAFANA_URL | ||
| value: {{ .Values.tools.grafana.url | quote }} | ||
| - name: GRAFANA_API_KEY | ||
| value: {{ .Values.tools.grafana.apiKey | quote }} |
Copilot
AI
Jul 31, 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 Grafana API key is being set as a plain environment variable. For better security, consider using a Kubernetes Secret and referencing it via valueFrom.secretKeyRef instead of exposing the API key in the deployment manifest.
| value: {{ .Values.tools.grafana.apiKey | quote }} | |
| valueFrom: | |
| secretKeyRef: | |
| name: grafana-api-key-secret | |
| key: apiKey |
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
yuval-k
left a comment
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.
approving based off of previous PR comments
|
hi, I am curious that is there any reason for switching from Autogen (https://kagent.dev/docs/kagent/concepts/architecture has yet updated) to ADK? |
This is the official PR to add support for ADK to kagent 🎉
This PR modifies many core parts of the system:
kagent-enginehas been removed