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_r280010382
##########
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:
> Even if we are using files to passing arguments to the GO instance why
does it have to be the arguments in instance config? Why not just write the
command line arguments in a file to begin with? Given we can ignore unknown
flags in GO, adding new flags shouldn't be a problem for BC.
I think the point of using file is to avoid using command args. it is not
about how do we pass those command args (using a file or specify them in the
run command). because command args is defined in the instance and they are
compiled into the final executable with the user code. once it is compiled, it
is compiled. you can't change it. it is different from python and java.
> I think there is a config you can set to to ignore unknown flags. Please
reference https://godoc.org/github.com/jessevdk/go-flags.
we can even write our own flags library to handle that if we really want to.
The point is - is there a standard on how do different languages parse and
treat *unknown flags* in the same way? If there is a standard, we can adopt it
and make all languages use the standard. If that is language specific or
library-specific, it is a red flag to me, especially in Go. Please remember the
instance is compiled with the user function as the final executable. It is much
difficult to fix bug or change library in our instance code.
Instead if we are using Yaml or Json, they are well-known standards and have
clear definitions on what are "unknown properties". I would be much comfortable
using them in such situation.
> A user can also copy and paste the command line into a text editor and
then parse it however he/she would like.
Well. I will say that the debug experience using a config file (configmap in
k8) is much much better than copying command line, paste it to a text editor,
reformat it, and parse it. Especially in a containerized environment.
> What about if we want to support another external scheduler like Nomad or
Mesos, it has it own way of mapping files into containers.
That's why we need a clear abstraction of languages and schedulers, as what
I have commented as the first comment in the PR. Commandline doesn't provide
you the right abstraction. It is messy as different languages have different
instance flags and use different parsing libraries.
If you are using yaml for defining the instance config, you get a clean and
unified way for describing what configuration that the instance uses to invoke
functions.
> While all of this is not impossible to do, my question is it really worth
it?
It is worth for many reasons:
- Better debug experience (as explained above)
- A unified way of defining the configuration that a (language) instance use
to invoke functions, within one scheduler. (as explained above)
- How to ship the instance config is left to schedulers (process, thread, or
k8s). This creates a better abstraction between languages and schedulers.
(aligned with the long term vision)
- Dynamic configuration loading if needed.
- Versioning the instance configs.
- ...
---
Anyway I think some of the discussions should be done in #4176.
@jerrypeng I would bringing the conversation back to help @wolfstudy move
forward with his change.
In summary, my take is to avoid using command lines in go instance since
that piece of code is shipped to be compiled with user function. Once it is
shipped, it is hard to change and we have to support it. If so, I would prefer
starting with current instance-config approach, since there is only one
command-line argument. It is easier to add more if the approach does creating
pain points in k8s runtime. But if we are adding all the command line
arguments, it becomes harder to remove them.
----------------------------------------------------------------
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
