xintongsong commented on code in PR #356:
URL: https://github.com/apache/flink-agents/pull/356#discussion_r2639084137
##########
.github/workflows/ci.yml:
##########
@@ -63,8 +63,24 @@ jobs:
with:
java-version: ${{ matrix.java-version }}
distribution: 'adopt'
+ - name: Install python
+ uses: actions/setup-python@v4
+ with:
+ python-version: '3.11'
+ - name: Install uv
+ uses: astral-sh/setup-uv@v4
+ with:
+ version: "latest"
+ - name: Install Python dependencies for MCP tests
+ run: |
+ cd python
+ uv sync --all-extras
- name: Run Java Tests
- run: tools/ut.sh -j
+ run: |
+ # Add Python venv to PATH so Java tests can find MCP dependencies
+ export PATH="${{ github.workspace }}/python/.venv/bin:$PATH"
+ export PYTHONPATH="${{ github.workspace
}}/python/.venv/lib/python3.11/site-packages:$PYTHONPATH"
+ tools/ut.sh -j
Review Comment:
IIUC, we need this to start an external mcp server, so that we can access
from the test cases? If that is the case, I'd suggest to move the test cases
that depends on external mcp server to the `it-java` pipeline. In theory, any
test case that interacts with an external system should be considered as
integration test or e2e test.
##########
api/src/main/java/org/apache/flink/agents/api/prompt/LocalPrompt.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.api.prompt;
+
+import org.apache.flink.agents.api.chat.messages.ChatMessage;
+import org.apache.flink.agents.api.chat.messages.MessageRole;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonCreator;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonProperty;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonTypeInfo;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+/**
+ * Local prompt implementation for language models.
+ *
+ * <p>This prompt implementation uses a local template that can be either a
string or a sequence of
+ * ChatMessage objects. The template supports placeholder substitution using
{variable} syntax.
+ *
+ * <p>Example usage:
+ *
+ * <pre>{@code
+ * // String template
+ * LocalPrompt prompt = new LocalPrompt("Hello {name}!");
+ * String result = prompt.formatString(Map.of("name", "World"));
+ * // result: "Hello World!"
+ *
+ * // Message template
+ * List<ChatMessage> messages = List.of(
+ * new ChatMessage(MessageRole.SYSTEM, "You are {role}"),
+ * new ChatMessage(MessageRole.USER, "Help me with {task}")
+ * );
+ * LocalPrompt prompt2 = new LocalPrompt(messages);
+ * List<ChatMessage> result2 = prompt2.formatMessages(
+ * MessageRole.SYSTEM,
+ * Map.of("role", "an assistant", "task", "coding")
+ * );
+ * }</pre>
+ */
+public class LocalPrompt extends Prompt {
Review Comment:
I'd suggest to make this a private class in `Prompt.java`. Users should not
be aware of the different implementations of `Prompt`. The should always use
`Prompt.fromText/Messages()` to create a prompt, rather than `new
LocalPrompt()`.
##########
api/src/main/java/org/apache/flink/agents/api/mcp/MCPServer.java:
##########
@@ -0,0 +1,423 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.api.mcp;
+
+import io.modelcontextprotocol.client.McpClient;
+import io.modelcontextprotocol.client.McpSyncClient;
+import
io.modelcontextprotocol.client.transport.HttpClientStreamableHttpTransport;
+import io.modelcontextprotocol.spec.McpSchema;
+import org.apache.flink.agents.api.chat.messages.ChatMessage;
+import org.apache.flink.agents.api.chat.messages.MessageRole;
+import org.apache.flink.agents.api.mcp.auth.ApiKeyAuth;
+import org.apache.flink.agents.api.mcp.auth.Auth;
+import org.apache.flink.agents.api.mcp.auth.BasicAuth;
+import org.apache.flink.agents.api.mcp.auth.BearerTokenAuth;
+import org.apache.flink.agents.api.resource.ResourceType;
+import org.apache.flink.agents.api.resource.SerializableResource;
+import org.apache.flink.agents.api.tools.ToolMetadata;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonCreator;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonIgnore;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonProperty;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper;
+
+import java.net.URI;
+import java.net.http.HttpRequest;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * Resource representing an MCP server and exposing its tools/prompts.
+ *
+ * <p>This is a logical container for MCP tools and prompts; it is not
directly invokable. It uses
+ * the official MCP Java SDK to communicate with MCP servers via HTTP/SSE.
+ *
+ * <p>Authentication is supported through the {@link Auth} interface with
multiple implementations:
+ *
+ * <ul>
+ * <li>{@link BearerTokenAuth} - For OAuth 2.0 and JWT tokens
+ * <li>{@link BasicAuth} - For username/password authentication
+ * <li>{@link ApiKeyAuth} - For API key authentication via custom headers
+ * </ul>
+ *
+ * <p>Example with OAuth authentication:
+ *
+ * <pre>{@code
+ * MCPServer server = MCPServer.builder("https://api.example.com/mcp";)
+ * .auth(new BearerTokenAuth("your-oauth-token"))
+ * .timeout(Duration.ofSeconds(30))
+ * .build();
+ *
+ * List<MCPTool> tools = server.listTools();
+ * server.close();
+ * }</pre>
+ *
+ * <p>Reference: <a
href="https://modelcontextprotocol.io/sdk/java/mcp-client";>MCP Java Client</a>
+ */
+public class MCPServer extends SerializableResource {
Review Comment:
@wenjin272, we should also fix this for the python api.
##########
api/src/main/java/org/apache/flink/agents/api/mcp/MCPServer.java:
##########
@@ -0,0 +1,423 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.api.mcp;
+
+import io.modelcontextprotocol.client.McpClient;
+import io.modelcontextprotocol.client.McpSyncClient;
+import
io.modelcontextprotocol.client.transport.HttpClientStreamableHttpTransport;
+import io.modelcontextprotocol.spec.McpSchema;
+import org.apache.flink.agents.api.chat.messages.ChatMessage;
+import org.apache.flink.agents.api.chat.messages.MessageRole;
+import org.apache.flink.agents.api.mcp.auth.ApiKeyAuth;
+import org.apache.flink.agents.api.mcp.auth.Auth;
+import org.apache.flink.agents.api.mcp.auth.BasicAuth;
+import org.apache.flink.agents.api.mcp.auth.BearerTokenAuth;
+import org.apache.flink.agents.api.resource.ResourceType;
+import org.apache.flink.agents.api.resource.SerializableResource;
+import org.apache.flink.agents.api.tools.ToolMetadata;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonCreator;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonIgnore;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonProperty;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper;
+
+import java.net.URI;
+import java.net.http.HttpRequest;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * Resource representing an MCP server and exposing its tools/prompts.
+ *
+ * <p>This is a logical container for MCP tools and prompts; it is not
directly invokable. It uses
+ * the official MCP Java SDK to communicate with MCP servers via HTTP/SSE.
+ *
+ * <p>Authentication is supported through the {@link Auth} interface with
multiple implementations:
+ *
+ * <ul>
+ * <li>{@link BearerTokenAuth} - For OAuth 2.0 and JWT tokens
+ * <li>{@link BasicAuth} - For username/password authentication
+ * <li>{@link ApiKeyAuth} - For API key authentication via custom headers
+ * </ul>
+ *
+ * <p>Example with OAuth authentication:
+ *
+ * <pre>{@code
+ * MCPServer server = MCPServer.builder("https://api.example.com/mcp";)
+ * .auth(new BearerTokenAuth("your-oauth-token"))
+ * .timeout(Duration.ofSeconds(30))
+ * .build();
+ *
+ * List<MCPTool> tools = server.listTools();
+ * server.close();
+ * }</pre>
+ *
+ * <p>Reference: <a
href="https://modelcontextprotocol.io/sdk/java/mcp-client";>MCP Java Client</a>
+ */
+public class MCPServer extends SerializableResource {
Review Comment:
Currently, the entire implementation of MCPServer and related tools/prompts
are in the `api` module, which is not ideal. We should keep the `api` module as
lightweight as possible, including only a minimum set of interfaces that users
need to depend on when writing their agent codes. I think we can move the
entire `api/mcp` directory into the the `integration` module. The only think
users need to see in the `api` module is the annotation.
##########
api/src/main/java/org/apache/flink/agents/api/prompt/LocalPrompt.java:
##########
@@ -0,0 +1,207 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.agents.api.prompt;
+
+import org.apache.flink.agents.api.chat.messages.ChatMessage;
+import org.apache.flink.agents.api.chat.messages.MessageRole;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonCreator;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonProperty;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonTypeInfo;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+/**
+ * Local prompt implementation for language models.
+ *
+ * <p>This prompt implementation uses a local template that can be either a
string or a sequence of
+ * ChatMessage objects. The template supports placeholder substitution using
{variable} syntax.
+ *
+ * <p>Example usage:
+ *
+ * <pre>{@code
+ * // String template
+ * LocalPrompt prompt = new LocalPrompt("Hello {name}!");
+ * String result = prompt.formatString(Map.of("name", "World"));
+ * // result: "Hello World!"
+ *
+ * // Message template
+ * List<ChatMessage> messages = List.of(
+ * new ChatMessage(MessageRole.SYSTEM, "You are {role}"),
+ * new ChatMessage(MessageRole.USER, "Help me with {task}")
+ * );
+ * LocalPrompt prompt2 = new LocalPrompt(messages);
+ * List<ChatMessage> result2 = prompt2.formatMessages(
+ * MessageRole.SYSTEM,
+ * Map.of("role", "an assistant", "task", "coding")
+ * );
+ * }</pre>
+ */
+public class LocalPrompt extends Prompt {
Review Comment:
And we should also use `Prompt.fromText/Messages()` in the doc sample codes,
examples and tests.
--
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]
