da-daken commented on code in PR #879:
URL: https://github.com/apache/flink-agents/pull/879#discussion_r3542088448


##########
plan/src/main/java/org/apache/flink/agents/plan/AgentPlan.java:
##########
@@ -316,31 +315,40 @@ private static ResourceDescriptor 
requireResourceDescriptor(
     }
 
     private void extractResource(ResourceType type, Method method) throws 
Exception {
-        extractResource(type, method, null);
+        extractResource(type, method, null, false);
     }
 
     private void extractResource(
             ResourceType type,
             Method method,
-            Function<ResourceDescriptor, ResourceDescriptor> 
descriptorDecorator)
+            Function<ResourceDescriptor, ResourceDescriptor> 
descriptorDecorator,
+            boolean isPython)
             throws Exception {
         String name = method.getName();
-        ResourceProvider provider;
         ResourceDescriptor descriptor = (ResourceDescriptor) 
method.invoke(null);
 
         descriptor =
                 descriptorDecorator != null ? 
descriptorDecorator.apply(descriptor) : descriptor;
 
-        if (PythonResourceWrapper.class.isAssignableFrom(
-                Class.forName(
-                        descriptor.getClazz(),
-                        true,
-                        Thread.currentThread().getContextClassLoader()))) {
-            provider = new PythonResourceProvider(name, type, descriptor);
-        } else {
-            provider = new JavaResourceProvider(name, type, descriptor);
+        addResourceProvider(createDescriptorResourceProvider(name, type, 
descriptor, isPython));
+    }
+
+    private ResourceProvider createDescriptorResourceProvider(
+            String name, ResourceType type, ResourceDescriptor descriptor) {
+        return createDescriptorResourceProvider(name, type, descriptor, false);
+    }
+
+    private ResourceProvider createDescriptorResourceProvider(
+            String name, ResourceType type, ResourceDescriptor descriptor, 
boolean isPython) {
+        if (isPython || isPythonResource(descriptor)) {
+            return new PythonResourceProvider(name, type, descriptor);
         }
-        addResourceProvider(provider);
+        return new JavaResourceProvider(name, type, descriptor);
+    }
+
+    private boolean isPythonResource(ResourceDescriptor descriptor) {
+        String pythonClazz = descriptor.getArgument("pythonClazz");

Review Comment:
   Thanks for your review! I think we should align on a single rule here. Since 
cross-language Python resources are required to declare pythonClazz, we 
probably should not leave other entry points open. Today, descriptors that take 
those alternate paths (e.g. non-empty module without pythonClazz) are 
classified as Java-side resources at plan-compilation time and fail at runtime. 
Tightening or closing those paths feels like a separate follow-up issue rather 
than something we need to solve in this PR—what do you think?



-- 
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]

Reply via email to