Add doc about Achieving Highly Reliable Asynchronous Communication with Apache RocketMQ #616
Conversation
Change-Id: I7b4d1f0cceed12da45f071d55d57c225d8f81fcb
Change-Id: I24ab4cc7f1741295b408785186505a23401e7307
Change-Id: Ia46efbd984e439e6499a9db16f2d4ac64c53b706
|
drizzle.zk seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Summary of ChangesHello @zk-drizzle, 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 significantly enhances the documentation by introducing a detailed guide on integrating Apache RocketMQ for highly reliable asynchronous communication within the AgentScope framework. It provides clear instructions and code examples for both client-side configuration of A2aAgents and server-side exposure of Agent services, enabling users to leverage RocketMQ's robust messaging capabilities for their agent-to-agent interactions. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds documentation for using Apache RocketMQ for asynchronous communication in both English and Chinese. The changes are well-structured and provide useful code examples. I've identified a few areas for improvement regarding code efficiency in the examples, consistency in formatting and naming, and minor formatting issues in the markdown. Addressing these points will enhance the clarity and quality of the documentation.
| private static AgentScopeAgentHandler agent(ReActAssistant.Builder builder) { | ||
| return new AgentScopeAgentHandler() { | ||
| @Override | ||
| public boolean isHealthy() { | ||
| return true; | ||
| } | ||
| @Override | ||
| public Flux<?> streamQuery(AgentRequest request, Object messages) { | ||
| ReActAgent agent = builder.build(); | ||
| StreamOptions streamOptions = StreamOptions.builder() | ||
| .eventTypes(EventType.REASONING, EventType.TOOL_RESULT) | ||
| .incremental(true) | ||
| .build(); | ||
|
|
||
| if (messages instanceof List<?>) { | ||
| return agent.stream((List<Msg>) messages, streamOptions); | ||
| } else if (messages instanceof Msg) { | ||
| return agent.stream((Msg) messages, streamOptions); | ||
| } else { | ||
| Msg msg = Msg.builder().role(MsgRole.USER).build(); | ||
| return agent.stream(msg, streamOptions); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return builder.build().getName(); | ||
| } | ||
|
|
||
| @Override | ||
| public String getDescription() { | ||
| return builder.build().getDescription(); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
There are a couple of issues in this agent helper method:
- The parameter type
ReActAssistant.Builderseems to be a typo and should likely beReActAgent.Builder. - The
builder.build()method is called ingetName()andgetDescription(), which is inefficient as it creates a new agent instance just to retrieve a property. It's better to build the agent once if its properties are needed in multiple places within the handler.
Here is a suggested refactoring to address these points:
//Build AgentScopeAgentHandler
private static AgentScopeAgentHandler agent(ReActAgent.Builder builder) {
// Build the agent once to get its name and description.
ReActAgent agentForInfo = builder.build();
return new AgentScopeAgentHandler() {
@Override
public boolean isHealthy() {
return true;
}
@Override
public Flux<?> streamQuery(AgentRequest request, Object messages) {
// A new agent instance is created for each query to ensure isolation.
ReActAgent agent = builder.build();
StreamOptions streamOptions = StreamOptions.builder()
.eventTypes(EventType.REASONING, EventType.TOOL_RESULT)
.incremental(true)
.build();
if (messages instanceof List<?>) {
return agent.stream((List<Msg>) messages, streamOptions);
} else if (messages instanceof Msg) {
return agent.stream((Msg) messages, streamOptions);
} else {
Msg msg = Msg.builder().role(MsgRole.USER).build();
return agent.stream(msg, streamOptions);
}
}
@Override
public String getName() {
return agentForInfo.getName();
}
@Override
public String getDescription() {
return agentForInfo.getDescription();
}
};
}| public static AgentScopeAgentHandler agent(ReActAssistant.Builder builder) { | ||
| return new AgentScopeAgentHandler() { | ||
| @Override | ||
| public boolean isHealthy() { | ||
| return true; | ||
| } | ||
| @Override | ||
| public Flux<?> streamQuery(AgentRequest request, Object messages) { | ||
| ReActAgent agent = builder.build(); | ||
| StreamOptions streamOptions = StreamOptions.builder() | ||
| .eventTypes(EventType.REASONING, EventType.TOOL_RESULT) | ||
| .incremental(true) | ||
| .build(); | ||
|
|
||
| if (messages instanceof List<?>) { | ||
| return agent.stream((List<Msg>) messages, streamOptions); | ||
| } else if (messages instanceof Msg) { | ||
| return agent.stream((Msg) messages, streamOptions); | ||
| } else { | ||
| Msg msg = Msg.builder().role(MsgRole.USER).build(); | ||
| return agent.stream(msg, streamOptions); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return builder.build().getName(); | ||
| } | ||
|
|
||
| @Override | ||
| public String getDescription() { | ||
| return builder.build().getDescription(); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
There are a couple of issues in this agent helper method:
- The parameter type
ReActAssistant.Builderseems to be a typo and should likely beReActAgent.Builder. - The
builder.build()method is called ingetName()andgetDescription(), which is inefficient as it creates a new agent instance just to retrieve a property. It's better to build the agent once if its properties are needed in multiple places within the handler.
Here is a suggested refactoring to address these points:
//构建AgentScopeAgentHandler
private static AgentScopeAgentHandler agent(ReActAgent.Builder builder) {
// Build the agent once to get its name and description.
ReActAgent agentForInfo = builder.build();
return new AgentScopeAgentHandler() {
@Override
public boolean isHealthy() {
return true;
}
@Override
public Flux<?> streamQuery(AgentRequest request, Object messages) {
// A new agent instance is created for each query to ensure isolation.
ReActAgent agent = builder.build();
StreamOptions streamOptions = StreamOptions.builder()
.eventTypes(EventType.REASONING, EventType.TOOL_RESULT)
.incremental(true)
.build();
if (messages instanceof List<?>) {
return agent.stream((List<Msg>) messages, streamOptions);
} else if (messages instanceof Msg) {
return agent.stream((Msg) messages, streamOptions);
} else {
Msg msg = Msg.builder().role(MsgRole.USER).build();
return agent.stream(msg, streamOptions);
}
}
@Override
public String getName() {
return agentForInfo.getName();
}
@Override
public String getDescription() {
return agentForInfo.getDescription();
}
};
}| | Parameter | Type | Description | Required | | ||
| |-----------|------|-----------|----| | ||
| | `accessKey` | String | RocketMQ AccessKey | NO | | ||
| | `secretKey` | String | RocketMQ SecretKey | NO | | ||
| | `workAgentResponseTopic` | String | LiteTopic | YES | | ||
| | `workAgentResponseGroupID` | String | The CID of the consumer subscribed to LiteTopic | YES | | ||
| | `rocketMQNamespace` | String | RocketMQ Namespace | NO | |
There was a problem hiding this comment.
The values in the Required column (YES/NO) are inconsistent with another table in this document (lines 352-356) which uses Yes/NO. To maintain consistency, please use Yes and No in all tables.
| | Parameter | Type | Description | Required | | |
| |-----------|------|-----------|----| | |
| | `accessKey` | String | RocketMQ AccessKey | NO | | |
| | `secretKey` | String | RocketMQ SecretKey | NO | | |
| | `workAgentResponseTopic` | String | LiteTopic | YES | | |
| | `workAgentResponseGroupID` | String | The CID of the consumer subscribed to LiteTopic | YES | | |
| | `rocketMQNamespace` | String | RocketMQ Namespace | NO | | |
| | Parameter | Type | Description | Required | | |
| | ------------------------ | ------ | -------------------------------------------- | -------- | | |
| | `accessKey` | String | RocketMQ AccessKey | No | | |
| | `secretKey` | String | RocketMQ SecretKey | No | | |
| | `workAgentResponseTopic` | String | LiteTopic | Yes | | |
| | `workAgentResponseGroupID` | String | The CID of the consumer subscribed to LiteTopic | Yes | | |
| | `rocketMQNamespace` | String | RocketMQ Namespace | No | |
| | Parameter | Type | Description | Required | | ||
| |-----------|------|-------------|----| | ||
| | `rocketmqEndpoint` | String | RocketMQ Endpoint | Yes | | ||
| | `rocketMQNamespace` | String | RocketMQ Namespace | NO | | ||
| | `bizTopic` | String | Normal Topic | Yes | |
There was a problem hiding this comment.
The values in the Required column are inconsistent. This table uses Yes and NO, while the previous one uses YES and NO. For consistency across the document, please use Yes and No.
| | Parameter | Type | Description | Required | | |
| |-----------|------|-------------|----| | |
| | `rocketmqEndpoint` | String | RocketMQ Endpoint | Yes | | |
| | `rocketMQNamespace` | String | RocketMQ Namespace | NO | | |
| | `bizTopic` | String | Normal Topic | Yes | | |
| | Parameter | Type | Description | Required | | |
| | ------------------ | ------ | ------------------- | -------- | | |
| | `rocketmqEndpoint` | String | RocketMQ Endpoint | Yes | | |
| | `rocketMQNamespace`| String | RocketMQ Namespace | No | | |
| | `bizTopic` | String | Normal Topic | Yes | |
| //Externally expose the AgentCard service based on RocketMQ communication | ||
| AgentInterface agentInterface = new AgentInterface(RocketMQA2AConstant.ROCKETMQ_PROTOCOL, buildRocketMQUrl()); | ||
| ConfigurableAgentCard agentCard = new ConfigurableAgentCard.Builder() | ||
| .url(buildRocketMQUrl()) |
There was a problem hiding this comment.
The buildRocketMQUrl() method is called again here. To improve readability and avoid redundant calls, it's better to call it once before line 362, store its result in a local variable, and reuse that variable here and on line 362.
| - **Apache RocketMQ Community** : https://github.com/apache/rocketmq | ||
| - **Apache RocketMQ A2A Asynchronous Communication Component** : https://github.com/apache/rocketmq-a2a |
There was a problem hiding this comment.
There are extra spaces before the colons in these list items. For consistency with the other items in this list and standard markdown formatting, please remove the spaces.
| - **Apache RocketMQ Community** : https://github.com/apache/rocketmq | |
| - **Apache RocketMQ A2A Asynchronous Communication Component** : https://github.com/apache/rocketmq-a2a | |
| - **Apache RocketMQ Community**: https://github.com/apache/rocketmq | |
| - **Apache RocketMQ A2A Asynchronous Communication Component**: https://github.com/apache/rocketmq-a2a |
| //对外开放基于Apache RocketMQ通信的AgentCard服务 | ||
| AgentInterface agentInterface = new AgentInterface(RocketMQA2AConstant.ROCKETMQ_PROTOCOL, buildRocketMQUrl()); | ||
| ConfigurableAgentCard agentCard = new ConfigurableAgentCard.Builder() | ||
| .url(buildRocketMQUrl()) |
There was a problem hiding this comment.
The buildRocketMQUrl() method is called again here. To improve readability and avoid redundant calls, it's better to call it once before line 359, store its result in a local variable, and reuse that variable here and on line 359.
|
|
||
| ```java | ||
| //构建DashScopeChatModel 用于调用LLM服务 | ||
| public static DashScopeChatModel dashScopeChatModel(String dashScopeApiKey) { |
There was a problem hiding this comment.
This method is declared public, but the corresponding method in the English documentation is private. To maintain consistency and encapsulate helper methods, it's better to use private. This also applies to agentBuilder (line 392) and agent (line 398) methods that follow.
| public static DashScopeChatModel dashScopeChatModel(String dashScopeApiKey) { | |
| private static DashScopeChatModel dashScopeChatModel(String dashScopeApiKey) { |
| - **Apache RocketMQ 社区** : https://github.com/apache/rocketmq | ||
| - **Apache RocketMQ A2A异步通信组件** : https://github.com/apache/rocketmq-a2a |
There was a problem hiding this comment.
There are extra spaces before the colons in these list items. For consistency with the other items in this list and standard markdown formatting, please remove the spaces.
| - **Apache RocketMQ 社区** : https://github.com/apache/rocketmq | |
| - **Apache RocketMQ A2A异步通信组件** : https://github.com/apache/rocketmq-a2a | |
| - **Apache RocketMQ 社区**: https://github.com/apache/rocketmq | |
| - **Apache RocketMQ A2A异步通信组件**: https://github.com/apache/rocketmq-a2a |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
AgentScope-Java Version
[The version of AgentScope-Java you are working on, e.g. 1.0.7, check your pom.xml dependency version or run
mvn dependency:tree | grep agentscope-parent:pom(only mac/linux)]Description
[Please describe the background, purpose, changes made, and how to test this PR]
Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test)