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_r298697956
##########
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
> for example, if I have a correct NAR package, but I provide a wrong path
(i.e. a typo in the url), what happened? do
That is checked before this logic here occurs. Either the user uploaded a
package or the user provided a link, both cases are handled prior to the
classloading logic.
> do I just get an exception with "Invalid source package"? The information
is not enough, no? because the exceptions are shallowed in both class loaders.
The reason the error message is "Invalid source package" is because if we
arrive in that part of the logic, we know that something is not right but it
could be couple things:
1. The user submitted a JAR but did not provide a classname
2. The user submitted a NAR that is incorrectly formatted
we can't exactly determine which one of the above 2 is the problem because
we are not currently assuming if a package is a JAR or a NAR and there is not
way for us to determine that. Perhaps in the future the better logic to have
to for us to assume if a package is a JAR or a NAR based on file suffix. I
think that is a reasonable assumption to make and will also make the logic and
error handling simpler and we will be able to return more sensible errors to
the user.
----------------------------------------------------------------
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
