eolivelli commented on a change in pull request #3678:
URL: https://github.com/apache/pulsar/pull/3678#discussion_r548482999



##########
File path: 
pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v3/SourceApiV3ResourceTest.java
##########
@@ -931,11 +932,16 @@ public void testUpdateSourceWithUrl() throws IOException {
 
         mockStatic(ConnectorUtils.class);
         doReturn(TwitterFireHose.class.getName()).when(ConnectorUtils.class);
-        ConnectorUtils.getIOSourceClass(any(NarClassLoader.class));
+        ConnectorUtils.getIOSourceClass(anyString(), 
any(NarClassLoader.class));
 
         mockStatic(org.apache.pulsar.functions.utils.Utils.class);
         
doReturn(String.class).when(org.apache.pulsar.functions.utils.Utils.class);
         org.apache.pulsar.functions.utils.Utils.getSourceType(anyString(), 
any(NarClassLoader.class));
+        
+        File mockedFile = mock(File.class);

Review comment:
       usually it is not a good practice to mock Java classes, it would be 
likely to break with future Java version, in this case you can create a dummy 
temporary file with the wanted file extension

##########
File path: 
pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/rest/api/v3/SourceApiV3ResourceTest.java
##########
@@ -105,7 +106,7 @@ public IObjectFactory getObjectFactory() {
     private static final String className = TwitterFireHose.class.getName();
     private static final int parallelism = 1;
     private static final String JAR_FILE_NAME = "pulsar-io-twitter.nar";
-    private static final String INVALID_JAR_FILE_NAME = 
"pulsar-io-cassandra.nar";
+    private static final String INVALID_JAR_FILE_NAME = 
"pulsar-io-cassandra-2.3.0-SNAPSHOT.nar";

Review comment:
       why are you changing this line ? does the version make a difference to 
us ?

##########
File path: 
pulsar-functions/utils/src/main/java/org/apache/pulsar/functions/utils/SourceConfigUtils.java
##########
@@ -210,7 +212,21 @@ public static ExtractedSourceDetails validate(SourceConfig 
sourceConfig, Path ar
         if (!isEmpty(sourceConfig.getClassName())) {
             sourceClassName = sourceConfig.getClassName();
             try {
-                classLoader = Utils.extractClassLoader(archivePath, 
functionPkgUrl, uploadedInputStreamAsFile);
+                String ext = null;
+                if (!isEmpty(functionPkgUrl)) {
+                    File file = Utils.extractFileFromPkg(functionPkgUrl);

Review comment:
       this file is only a temporary file, we need it only for the extension.
   can we remove the file as soon as we do not need it anymore ?
   it would be also better to not extract the file but only to scan for the 
file name




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


Reply via email to