-
Notifications
You must be signed in to change notification settings - Fork 892
Python: ADR for create/get agent API #2618
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
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 introduces an ADR (Architecture Decision Record) that documents the misalignment between .NET and Python implementations of the create/get agent API, proposing a solution to achieve API parity.
- Documents current API differences between .NET and Python agent creation/retrieval methods
- Proposes adding missing
get_agentand enhancedcreate_agentmethods to Python implementations - Provides comprehensive comparison table of current .NET provider implementations
eavanvalkenburg
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.
I can get behind the principle, my only concern is that the overridden create agent methods will need to be async to work and that's not great
| Python: | ||
|
|
||
| ```python | ||
| agent1 = AIProjectClient(...).get_agent(agent_sdk_instance) # Creates a local ChatAgent instance |
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.
what exactly is the agent_sdk_instance here?
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.
I will update this to be more descriptive, but basically it's an instance of agent that is coming from underlying SDK like AgentReference from Azure AI or Assistant from OpenAI Assistants.
| ```python | ||
| agent1 = AIProjectClient(...).get_agent(agent_sdk_instance) # Creates a local ChatAgent instance | ||
| agent2 = AIProjectClient(...).get_agent(agent_name) # Fetches agent data, creates a local ChatAgent instance | ||
| agent3 = AIProjectClient(...).create_agent(...) # Creates a remote agent, returns a local ChatAgent instance |
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.
This makes sense, especially if create_agent becomes a async call, and then we should keep this on all clients, but the behavior should be different, for agents with a agent-like service behind it, is does the work, for agents that don't have that it does the same as get_agent
|
|
||
| ### agent-framework-azure-ai (both V1 and V2) | ||
|
|
||
| - Add a `get_agent` method that accepts an underlying SDK agent instance and creates a local instance of `ChatAgent`. |
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.
what about as_agent, get still implies some kind of network activity
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.
I don't have a strong preference, but get will be consistent with .NET naming (at least for now), so that's why I like it more.
Motivation and Context
There is a misalignment between the create/get agent API in the .NET and Python implementations. This PR contains ADR to compare APIs in both languages to decide what needs to be added.
Contribution Checklist