jerrypeng commented on a change in pull request #4577: fix issue when 
submitting NAR via file url
URL: https://github.com/apache/pulsar/pull/4577#discussion_r297419829
 
 

 ##########
 File path: 
pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/SinkConfigUtils.java
 ##########
 @@ -318,79 +322,89 @@ public static ExtractedSinkDetails validate(SinkConfig 
sinkConfig, Path archiveP
             throw new IllegalArgumentException("Sink timeout must be a 
positive number");
         }
 
-        String sinkClassName;
-        final Class<?> typeArg;
-        final ClassLoader classLoader;
-        if (!isEmpty(sinkConfig.getClassName())) {
-            sinkClassName = sinkConfig.getClassName();
-            // We really don't know if we should use nar class loader or 
regular classloader
-            ClassLoader jarClassLoader = null;
-            ClassLoader narClassLoader = null;
-            try {
-                jarClassLoader = 
FunctionCommon.extractClassLoader(archivePath, uploadedInputStreamAsFile);
-            } catch (Exception e) {
+
+        Class<?> typeArg;
+        ClassLoader classLoader;
+        String sinkClassName = sinkConfig.getClassName();
+        ClassLoader jarClassLoader = null;
+        ClassLoader narClassLoader = null;
+        try {
+            jarClassLoader = FunctionCommon.extractClassLoader(archivePath, 
sinkPackageFile);
+        } catch (Exception e) {
 
 Review comment:
   @sijie 
   
   > first of all, it is going to become very hard to help people to understand 
the problem if we don't log any errors.
   
   We are logging errors in the code. Please read further down the code.
   
   The JAR classloader will also be non-null.  The only errors can happen when 
trying to load as a NAR.  Let just say we log these NAR errors regardless of 
whether a user submitted a NAR of not.  Later and cluster admin comes back and 
looks at the logs and see all these errors loading a NAR. At in the best case 
scenario the admin will just be confused and at worse help him to misdiagnose 
the problem and perform some action to make things worse.  
   
   Logging misleading information will just create more confusion.  Perhaps the 
logic of this code

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to