weiqingy commented on code in PR #879:
URL: https://github.com/apache/flink-agents/pull/879#discussion_r3547623127
##########
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:
Agreed — making `pythonClazz` the single rule and closing the alternate
paths is cleaner than documenting the assumption, and it belongs in its own
change. Worth a follow-up issue though: those descriptors classify as Java at
compile time and only fail at runtime, so sharpening that into a clearer
compile-time error would be a good tracker. Otherwise LGTM — I'll leave the
final call to the maintainers.
--
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]
