-
Notifications
You must be signed in to change notification settings - Fork 151
Initial A2A Integration #218
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 this. Let's get the changes to pyproject.toml that give the feature options for otel in it's own PR so we can get this deployed ASAP and unblock A2A. Then follow up with the implementation stuff? Thoughts? |
@mikegc-aws yes great callout, i'll extract that into a separate PR and ship that first. |
b8e12bc
to
1c046c1
Compare
@@ -108,7 +108,7 @@ format-fix = [ | |||
] | |||
lint-check = [ | |||
"ruff check", | |||
"mypy -p src" | |||
"mypy -p src --exclude src/strands/multiagent" |
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's this needed for?
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.
Oh, is this because of the dependency conflict stuff? If so, can you add comments throughout the file indicating that?
Also, do we have a tracking issue for the dependency issue. I think the long-term fix is to remove the http exporter dependency from strands?
@@ -216,6 +216,9 @@ def __init__( | |||
record_direct_tool_call: bool = True, | |||
load_tools_from_directory: bool = True, | |||
trace_attributes: Optional[Mapping[str, AttributeValue]] = None, | |||
*, | |||
name: str | None = None, |
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.
Can we switch to Optional[str]
for consistency elsewhere? (I like this syntax more TBH, but we use Optional everywhere else - issue to switch the library over maybe?)
from ...agent.agent import Agent as SAAgent | ||
from .executor import StrandsA2AExecutor | ||
|
||
log = logging.getLogger(__name__) |
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.
nit -> I think our convention is logger =
log = logging.getLogger(__name__) | ||
|
||
|
||
class A2AAgent: |
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 know we also wanted the concept of an A2A agent that Strands can talk to like a StrandsAgent - what were we thinking with naming there?
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.
As is, this seems more like a A2AAgentAdapter
or something. A2AAgent
makes me think it's a native A2AAgent that I can talk to like an Agent
agent: SAAgent, | ||
*, | ||
# AgentCard | ||
host: str = "0.0.0", |
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.
Should this have a 4th zero or is this not an ip?
agent: The Strands Agent to wrap with A2A compatibility. | ||
name: The name of the agent, used in the AgentCard. | ||
description: A description of the agent's capabilities, used in the AgentCard. | ||
host: The hostname or IP address to bind the A2A server to. Defaults to "localhost". |
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.
Right now it hosts to "0.0.0" actually
class A2AAgent: | ||
"""A2A-compatible wrapper for Strands Agent.""" | ||
|
||
def __init__( |
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.
Is there anything we can do to mark this as experiemental/in-development? Maybe log an info message in the constructor or something?
""" | ||
return A2AFastAPIApplication(agent_card=self.public_agent_card, http_handler=self.request_handler).build() | ||
|
||
def serve(self, app_type: Literal["fastapi", "starlette"] = "starlette", **kwargs: Any) -> None: |
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.
Is this required by A2A or is this our thing?
Now that we have this concept, I sort of want it for Strands natively - or does that not make sense now that A2A exists?
if "text" in content_block: | ||
await event_queue.enqueue_event(new_agent_text_message(content_block["text"])) | ||
|
||
async def cancel(self, context: RequestContext, event_queue: EventQueue) -> None: |
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.
Can you add this as a use case for #81
@@ -216,6 +216,9 @@ def __init__( | |||
record_direct_tool_call: bool = True, | |||
load_tools_from_directory: bool = True, | |||
trace_attributes: Optional[Mapping[str, AttributeValue]] = None, | |||
*, | |||
name: str | None = None, | |||
description: str | None = None, |
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 PR description has this example:
# Log that we're starting
logging.info("Starting A2A server with root logger")
strands_agent = Agent(model="us.anthropic.claude-3-haiku-20240307-v1:0", callback_handler=None)
strands_a2a_agent = A2AAgent(agent=strands_agent, name="Hello World Agent", description="Just a hello world agent")
strands_a2a_agent.serve()
But per this code, the name/description are now part of Agent correct? Could the PR description be updated?
Description
This PR seeks to integrate Google's A2A protocol into Strands agents
Related Issues
Documentation PR
N/A
Type of Change
Limitations and Next Steps
A2AAgentClass
which currently only supportsmessages
.opentelemetry-exporter-otlp-proto-http
anda2a-sdk
dependencies.a2a-sdk
relies onprotobuf>=6.3.1
, butopentelemetry-exporter-otlp-proto-http
requires<6.0
. This warranted the dependency changes topyproject.toml
and implementation changes totracer.py
. I plan on documenting the conflict and submitting issues to both the opentelemetry-exporter-otlp-proto-http and the a2a-sdk repo.Testing
sample server:
a2a_server.py
sample client
a2a_client.py
Run
python a2a_server.py
andpython a2a_client.py
in separate terminal panes and observe output.Standard Testing
hatch fmt --linter
hatch fmt --formatter
hatch test --all
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.