xintongsong commented on code in PR #452:
URL: https://github.com/apache/flink-agents/pull/452#discussion_r2706617645
##########
api/src/main/java/org/apache/flink/agents/api/resource/Constant.java:
##########
@@ -81,5 +81,5 @@ public class Constant {
"org.apache.flink.agents.integrations.vectorstores.elasticsearch.ElasticsearchVectorStore";
// MCP
- public static String MCP_SERVER =
"org.apache.flink.agents.integrations.mcp.MCPServer";
+ public static String MCP_SERVER = "unified.class.path.MCPServer";
Review Comment:
```suggestion
public static String MCP_SERVER = "DECIDE_IN_RUNTIME_MCPServer";
```
##########
api/src/main/java/org/apache/flink/agents/api/annotation/MCPServer.java:
##########
@@ -58,11 +58,15 @@
* }</pre>
*
* <p>This is the Java equivalent of Python's {@code @mcp_server} decorator.
- *
- * @see org.apache.flink.agents.integrations.mcp.MCPServer
- * @see org.apache.flink.agents.integrations.mcp.MCPTool
- * @see org.apache.flink.agents.integrations.mcp.MCPPrompt
*/
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
-public @interface MCPServer {}
+public @interface MCPServer {
+ /**
+ * The version of MCPServer to be used. Default is "python" for JDK 16 and
below, and "java" for
+ * JDK 17+.
+ *
+ * @return the version value
+ */
+ String version() default "java";
Review Comment:
```suggestion
String lang() default "java";
```
##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -304,6 +344,13 @@ private void extractResource(ResourceType type, Method
method) throws Exception
String name = method.getName();
ResourceProvider provider;
ResourceDescriptor descriptor = (ResourceDescriptor)
method.invoke(null);
+ if (type == MCP_SERVER) {
+ descriptor =
+ new ResourceDescriptor(
+ descriptor.getModule(),
+ PythonMCPServer.class.getName(),
Review Comment:
It's against intuition that `extractResource` would assume using python mcp
server. I think we can introduce a descriptor decorator for this method.
##########
api/src/main/java/org/apache/flink/agents/api/annotation/MCPServer.java:
##########
@@ -58,11 +58,15 @@
* }</pre>
*
* <p>This is the Java equivalent of Python's {@code @mcp_server} decorator.
- *
- * @see org.apache.flink.agents.integrations.mcp.MCPServer
- * @see org.apache.flink.agents.integrations.mcp.MCPTool
- * @see org.apache.flink.agents.integrations.mcp.MCPPrompt
*/
@Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME)
-public @interface MCPServer {}
+public @interface MCPServer {
+ /**
+ * The version of MCPServer to be used. Default is "python" for JDK 16 and
below, and "java" for
+ * JDK 17+.
+ *
+ * @return the version value
+ */
+ String version() default "java";
Review Comment:
The default value should be `null` or something like `auto`, so that we can
differentiate whether it's explicitly configured by users, or automatically
decided by the framework. If explicitly configured `java` and the jdk version
doesn't meet the requirements, we should fail.
##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -133,8 +141,40 @@ public AgentPlan(Agent agent, AgentConfiguration config)
throws Exception {
this.config = config;
}
- public void setPythonResourceAdapter(PythonResourceAdapter adapter) {
+ public void setPythonResourceAdapter(PythonResourceAdapter adapter) throws
Exception {
this.pythonResourceAdapter = adapter;
+ Map<String, ResourceProvider> servers =
resourceProviders.get(MCP_SERVER);
+ if (servers != null) {
+ for (ResourceProvider provider : servers.values()) {
+ if (provider instanceof PythonResourceProvider) {
+ ((PythonResourceProvider)
provider).setPythonResourceAdapter(adapter);
+ PythonMCPServer server =
+ (PythonMCPServer)
+ provider.provide(
+ (String anotherName, ResourceType
anotherType) -> {
+ try {
+ return this.getResource(
+ anotherName,
anotherType);
+ } catch (Exception e) {
+ throw new
RuntimeException(e);
+ }
+ });
+ List<PythonMCPTool> pythonMCPTools = server.listTools();
+ for (PythonMCPTool pythonMCPTool : pythonMCPTools) {
+ resourceCache
+ .computeIfAbsent(TOOL, k -> new
ConcurrentHashMap<>())
+ .put(pythonMCPTool.getName(), pythonMCPTool);
+ }
+
+ List<PythonMCPPrompt> pythonMCPPrompts =
server.listPrompts();
+ for (PythonMCPPrompt pythonMCPPrompt : pythonMCPPrompts) {
+ resourceCache
+ .computeIfAbsent(PROMPT, k -> new
ConcurrentHashMap<>())
+ .put(pythonMCPPrompt.getName(),
pythonMCPPrompt);
+ }
+ }
+ }
+ }
Review Comment:
Minor: unnecessary deep indention
##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -448,18 +500,18 @@ private void extractResourceProvidersFromAgent(Agent
agent) throws Exception {
extractResource(ResourceType.EMBEDDING_MODEL_CONNECTION,
method);
} else if (method.isAnnotationPresent(VectorStore.class)) {
extractResource(ResourceType.VECTOR_STORE, method);
- } else if (Modifier.isStatic(method.getModifiers())) {
- // Check for MCPServer annotation using reflection to support
Java 11 without MCP
- try {
- Class<?> mcpServerAnnotation =
-
Class.forName("org.apache.flink.agents.api.annotation.MCPServer");
- if (method.isAnnotationPresent(
- (Class<? extends java.lang.annotation.Annotation>)
- mcpServerAnnotation)) {
- extractMCPServer(method);
- }
- } catch (ClassNotFoundException e) {
- // MCP annotation not available (Java 11 build), skip MCP
processing
+ } else if (method.isAnnotationPresent(MCPServer.class)) {
+ // Check the MCPServer annotation version to determine which
version to use.
+ MCPServer MCPServerAnnotation =
method.getAnnotation(MCPServer.class);
+ String version = MCPServerAnnotation.version();
+ int javaVersion = Runtime.version().feature();
+ if (version.equalsIgnoreCase("java") && javaVersion >= 17) {
+ extractMCPServer(method);
Review Comment:
`extractMCPServer` is confusing. I'd suggest the name `extractJavaMCPServer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
