sijie commented on a change in pull request #4174: [go function] support 
localrun and cluster mode for go function
URL: https://github.com/apache/pulsar/pull/4174#discussion_r279610291
 
 

 ##########
 File path: 
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java
 ##########
 @@ -89,31 +91,180 @@
             if (StringUtils.isNotEmpty(extraDependenciesDir)) {
                 args.add("PYTHONPATH=${PYTHONPATH}:" + extraDependenciesDir);
             }
+        } else if (instanceConfig.getFunctionDetails().getRuntime() == 
Function.FunctionDetails.Runtime.GO) {
+            //no-op
         }
 
         return args;
     }
 
+    /**
+     *
+     * Different from python and java function, Go function uploads a complete 
executable file(including:
+     * instance file + user code file). Its parameter list is provided to the 
broker in the form of a yaml file,
+     * the advantage of this approach is that backward compatibility is 
guaranteed.
 
 Review comment:
   I would suggest expanding more details on what does the backward 
compatibility mean here.
   
   If I understand this correctly, in Java and Python the instance is managed 
by broker (or function worker) so the changes in command line is under control; 
but in Go the instance is compiled with the user function, so pulsar doesn't 
have the control what instance is used in the function. Hence in order to 
support BC for go function, we can't dynamically add more commandline 
arguments. Using an instance config to pass the parameters from function worker 
to go instance is the best way for maintaining the BC. 
   
   Am I understanding this correctly?

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