Re: [PR] [Feature][runtime] Support the use of Python ChatModel in Java [flink-agents]

["wenjin272 (via GitHub)" <gi...@apache.org>]

wenjin272 commented on code in PR #314:
URL: https://github.com/apache/flink-agents/pull/314#discussion_r2525608647


##########
plan/src/main/java/org/apache/flink/agents/plan/resourceprovider/PythonResourceProvider.java:
##########
@@ -46,6 +54,25 @@ public PythonResourceProvider(
         this.module = module;
         this.clazz = clazz;
         this.kwargs = kwargs;
+        this.descriptor = null;
+    }
+
+    public PythonResourceProvider(String name, ResourceType type, ResourceDescriptor descriptor) {
+        super(name, type);
+        module = descriptor.getArgument("module");
+        if (module == null || module.isEmpty()) {
+            throw new IllegalArgumentException("module should not be null or empty.");
+        }
+        clazz = descriptor.getArgument("clazz");
+        if (clazz == null || clazz.isEmpty()) {
+            throw new IllegalArgumentException("clazz should not be null or empty.");
+        }
+        this.kwargs = new HashMap<>(descriptor.getInitialArguments());

Review Comment:
   This kwargs may need remove `module` and `clazz`.
   
   Or we can just add a new implementation `PythonResourceDescriptor`, which has additional property `pythonMoudel` and `pythonClass`. User can directly use it when declare a resource in Agent.



##########
plan/src/test/resources/resource_providers/python_resource_provider.json:
##########
@@ -3,6 +3,7 @@
   "type" : "chat_model",
   "module" : "flink_agents.plan.tests.test_resource_provider",
   "clazz" : "MockChatModelImpl",
+  "descriptor" :null,

Review Comment:
   This json is serialized from python agent plan, but `PythonResourceProvider` in python won't generate `descriptor` field. 



##########
plan/src/main/java/org/apache/flink/agents/plan/serializer/ResourceProviderJsonDeserializer.java:
##########
@@ -76,6 +76,15 @@ public ResourceProvider deserialize(
     private PythonResourceProvider deserializePythonResourceProvider(JsonNode node) {
         String name = node.get("name").asText();
         String type = node.get("type").asText();
+        try {
+            ResourceDescriptor descriptor =
+                    mapper.treeToValue(node.get("descriptor"), ResourceDescriptor.class);
+            if (descriptor != null) {

Review Comment:
   We can check the `descriptor` field existence rather than if the field is null.



##########
plan/src/main/java/org/apache/flink/agents/plan/resourceprovider/PythonResourceProvider.java:
##########
@@ -63,11 +90,22 @@ public Map<String, Object> getKwargs() {
     @Override
     public Resource provide(BiFunction<String, ResourceType, Resource> getResource)
             throws Exception {
-        // TODO: Implement Python resource creation logic
-        // This would typically involve calling into Python runtime to create the
-        // resource
-        throw new UnsupportedOperationException(
-                "Python resource creation not yet implemented in Java runtime");
+        if (pythonResourceAdapter != null) {
+            Class<?> clazz = Class.forName(descriptor.getClazz());
+            Map<String, Object> kwargs = new HashMap<>(descriptor.getInitialArguments());

Review Comment:
   Why not use class data member but create `clazz` and `kwargs` again here? 



##########
plan/src/main/java/org/apache/flink/agents/plan/resourceprovider/PythonResourceProvider.java:
##########
@@ -46,6 +54,25 @@ public PythonResourceProvider(
         this.module = module;
         this.clazz = clazz;
         this.kwargs = kwargs;
+        this.descriptor = null;

Review Comment:
   Should we add `descriptor` field for  `PythonResourceProvider` deserialized from agent plan declared using python api?



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org