shibd commented on code in PR #15693:
URL: https://github.com/apache/pulsar/pull/15693#discussion_r886586299


##########
pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/functions/FunctionUtils.java:
##########
@@ -98,24 +95,29 @@ public static Functions searchForFunctions(String 
functionsDirectory, boolean al
         try (DirectoryStream<Path> stream = Files.newDirectoryStream(path, 
"*.nar")) {
             for (Path archive : stream) {
                 try {
-                    FunctionDefinition cntDef = 
FunctionUtils.getFunctionDefinition(archive.toString());
+
+                    NarClassLoader ncl = NarClassLoaderBuilder.builder()
+                            .narFile(new File(archive.toString()))
+                            .build();
+
+                    FunctionArchive.FunctionArchiveBuilder 
functionArchiveBuilder = FunctionArchive.builder();
+                    FunctionDefinition cntDef = 
FunctionUtils.getFunctionDefinition(ncl);
                     log.info("Found function {} from {}", cntDef, archive);
-                    log.error(cntDef.getName());
-                    log.error(cntDef.getFunctionClass());
+
+                    functionArchiveBuilder.archivePath(archive);
+
+                    functionArchiveBuilder.classLoader(ncl);
+                    functionArchiveBuilder.functionDefinition(cntDef);
+
                     if (alwaysPopulatePath || 
!StringUtils.isEmpty(cntDef.getFunctionClass())) {
-                        functions.functions.put(cntDef.getName(), archive);
+                        functions.put(cntDef.getName(), 
functionArchiveBuilder.build());

Review Comment:
   When `alwayPopulatePath` == false or `cntDef.getFunctionClass()` is empty, 
function may be result empty. This is not the same as the original logic.  
Please check if there is a problem, and add a unit test to cover it.



##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdFunctions.java:
##########
@@ -643,14 +643,18 @@ void processArguments() throws Exception {
 
         protected void validateFunctionConfigs(FunctionConfig functionConfig) {
             // go doesn't need className
-            if (functionConfig.getPy() != null || functionConfig.getJar() != 
null) {
+            if (functionConfig.getPy() != null
+                    || (functionConfig.getJar() != null && 
!functionConfig.getJar().startsWith("builtin://"))) {

Review Comment:
   Add unit test coverd it.



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