You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2023/10/08 16:42:57 UTC

[PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

pan3793 opened a new pull request, #4668:
URL: https://github.com/apache/zeppelin/pull/4668

   ### What is this PR for?
   
   Clean up unreachable code for legacy Spark 1.x and 2.x versions, since Zeppelin only supports Spark 3.2 ~ 3.4 now.
   
   ### What type of PR is it?
   
   Improvement
   
   ### Todos
   * [ ] - Task
   
   ### What is the Jira issue?
   
   ZEPPELIN-5946
   
   ### How should this be tested?
   
   Pass CI
   
   ### Screenshots (if appropriate)
   
   ### Questions:
   * Does the license files need to update? No.
   * Is there breaking changes for older versions? No.
   * Does this needs documentation? No.
   


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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#discussion_r1350366811


##########
python/src/main/java/org/apache/zeppelin/python/PythonUtils.java:
##########
@@ -37,35 +36,22 @@ public class PythonUtils {
   public static GatewayServer createGatewayServer(Object entryPoint,
                                                   String serverAddress,
                                                   int port,
-                                                  String secretKey,
-                                                  boolean useAuth) throws IOException {
-    LOGGER.info("Launching GatewayServer at " + serverAddress + ":" + port +
-        ", useAuth: " + useAuth);
-    if (useAuth) {
-      try {
-        Class clz = Class.forName("py4j.GatewayServer$GatewayServerBuilder", true,
-            Thread.currentThread().getContextClassLoader());
-        Object builder = clz.getConstructor(Object.class).newInstance(entryPoint);
-        builder.getClass().getMethod("authToken", String.class).invoke(builder, secretKey);
-        builder.getClass().getMethod("javaPort", int.class).invoke(builder, port);
-        builder.getClass().getMethod("javaAddress", InetAddress.class).invoke(builder,
-            InetAddress.getByName(serverAddress));
-        builder.getClass()
-            .getMethod("callbackClient", int.class, InetAddress.class, String.class)
-            .invoke(builder, port, InetAddress.getByName(serverAddress), secretKey);
-        return (GatewayServer) builder.getClass().getMethod("build").invoke(builder);
-      } catch (Exception e) {
-        throw new IOException(e);
-      }
-    } else {
-      return new GatewayServer(entryPoint,
-          port,
-          GatewayServer.DEFAULT_PYTHON_PORT,
-          InetAddress.getByName(serverAddress),
-          InetAddress.getByName(serverAddress),
-          GatewayServer.DEFAULT_CONNECT_TIMEOUT,
-          GatewayServer.DEFAULT_READ_TIMEOUT,
-          (List) null);
+                                                  String secretKey) throws IOException {
+    LOGGER.info("Launching GatewayServer at {}:{}", serverAddress, port);
+    try {
+      Class clz = Class.forName("py4j.GatewayServer$GatewayServerBuilder", true,

Review Comment:
   Can you document here why reflection is used here.



##########
python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java:
##########
@@ -116,7 +115,6 @@ public void open() throws InterpreterException {
     }
 
     try {
-      this.usePy4jAuth = Boolean.parseBoolean(getProperty("zeppelin.py4j.useAuth", "true"));

Review Comment:
   :heavy_check_mark: 



##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncher.java:
##########
@@ -294,37 +292,17 @@ private String detectSparkScalaVersion(String sparkHome, Map<String, String> env
   }
 
   private String detectSparkScalaVersionByReplClass(String sparkHome) throws Exception {
-    File sparkLibFolder = new File(sparkHome + "/lib");
-    if (sparkLibFolder.exists()) {
-      // spark 1.6 if spark/lib exists
-      File[] sparkAssemblyJars = new File(sparkHome + "/lib").listFiles(new FilenameFilter() {
-        @Override
-        public boolean accept(File dir, String name) {
-          return name.contains("spark-assembly");
-        }
-      });
-      if (sparkAssemblyJars.length == 0) {
-        throw new Exception("No spark assembly file found in SPARK_HOME: " + sparkHome);
-      }
-      if (sparkAssemblyJars.length > 1) {
-        throw new Exception("Multiple spark assembly file found in SPARK_HOME: " + sparkHome);
-      }
-      try (URLClassLoader urlClassLoader = new URLClassLoader(new URL[]{sparkAssemblyJars[0].toURI().toURL()});){
-        urlClassLoader.loadClass("org.apache.spark.repl.SparkCommandLine");
-        return "2.10";
-      } catch (ClassNotFoundException e) {
-        return "2.11";
-      }
+    File sparkJarsFolder = new File(sparkHome + "/jars");
+    boolean sparkRepl212Exists =
+            Stream.of(sparkJarsFolder.listFiles()).anyMatch(file -> file.getName().contains("spark-repl_2.12"));
+    boolean sparkRepl213Exists =
+            Stream.of(sparkJarsFolder.listFiles()).anyMatch(file -> file.getName().contains("spark-repl_2.13"));
+    if (sparkRepl212Exists) {

Review Comment:
   I like this approach. Do we want to throw an exception as soon as multiple versions are found?



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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#issuecomment-1755514481

   @jongyoul the failed CI is irrelevant, could you merge this one? I'm going to rebase the #4665 once this gets in


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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#issuecomment-1752262901

   cc @jongyoul @Reamer


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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer merged PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668


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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#discussion_r1352304028


##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncher.java:
##########
@@ -294,37 +292,17 @@ private String detectSparkScalaVersion(String sparkHome, Map<String, String> env
   }
 
   private String detectSparkScalaVersionByReplClass(String sparkHome) throws Exception {
-    File sparkLibFolder = new File(sparkHome + "/lib");
-    if (sparkLibFolder.exists()) {
-      // spark 1.6 if spark/lib exists
-      File[] sparkAssemblyJars = new File(sparkHome + "/lib").listFiles(new FilenameFilter() {
-        @Override
-        public boolean accept(File dir, String name) {
-          return name.contains("spark-assembly");
-        }
-      });
-      if (sparkAssemblyJars.length == 0) {
-        throw new Exception("No spark assembly file found in SPARK_HOME: " + sparkHome);
-      }
-      if (sparkAssemblyJars.length > 1) {
-        throw new Exception("Multiple spark assembly file found in SPARK_HOME: " + sparkHome);
-      }
-      try (URLClassLoader urlClassLoader = new URLClassLoader(new URL[]{sparkAssemblyJars[0].toURI().toURL()});){
-        urlClassLoader.loadClass("org.apache.spark.repl.SparkCommandLine");
-        return "2.10";
-      } catch (ClassNotFoundException e) {
-        return "2.11";
-      }
+    File sparkJarsFolder = new File(sparkHome + "/jars");
+    boolean sparkRepl212Exists =
+            Stream.of(sparkJarsFolder.listFiles()).anyMatch(file -> file.getName().contains("spark-repl_2.12"));
+    boolean sparkRepl213Exists =
+            Stream.of(sparkJarsFolder.listFiles()).anyMatch(file -> file.getName().contains("spark-repl_2.13"));
+    if (sparkRepl212Exists) {

Review Comment:
   added check code in https://github.com/apache/zeppelin/pull/4668/commits/0c30040a2766221542b9363abbd53974666bb2c0



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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#discussion_r1352292918


##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncher.java:
##########
@@ -294,37 +292,17 @@ private String detectSparkScalaVersion(String sparkHome, Map<String, String> env
   }
 
   private String detectSparkScalaVersionByReplClass(String sparkHome) throws Exception {
-    File sparkLibFolder = new File(sparkHome + "/lib");
-    if (sparkLibFolder.exists()) {
-      // spark 1.6 if spark/lib exists
-      File[] sparkAssemblyJars = new File(sparkHome + "/lib").listFiles(new FilenameFilter() {
-        @Override
-        public boolean accept(File dir, String name) {
-          return name.contains("spark-assembly");
-        }
-      });
-      if (sparkAssemblyJars.length == 0) {
-        throw new Exception("No spark assembly file found in SPARK_HOME: " + sparkHome);
-      }
-      if (sparkAssemblyJars.length > 1) {
-        throw new Exception("Multiple spark assembly file found in SPARK_HOME: " + sparkHome);
-      }
-      try (URLClassLoader urlClassLoader = new URLClassLoader(new URL[]{sparkAssemblyJars[0].toURI().toURL()});){
-        urlClassLoader.loadClass("org.apache.spark.repl.SparkCommandLine");
-        return "2.10";
-      } catch (ClassNotFoundException e) {
-        return "2.11";
-      }
+    File sparkJarsFolder = new File(sparkHome + "/jars");
+    boolean sparkRepl212Exists =
+            Stream.of(sparkJarsFolder.listFiles()).anyMatch(file -> file.getName().contains("spark-repl_2.12"));
+    boolean sparkRepl213Exists =
+            Stream.of(sparkJarsFolder.listFiles()).anyMatch(file -> file.getName().contains("spark-repl_2.13"));
+    if (sparkRepl212Exists) {

Review Comment:
   In the old code, an exception is thrown for unclean installation. I would still like to see such error handling in the code.
   ```
         if (sparkAssemblyJars.length > 1) {
           throw new Exception("Multiple spark assembly file found in SPARK_HOME: " + sparkHome);
         }
   ```



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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#discussion_r1350606484


##########
python/src/main/java/org/apache/zeppelin/python/PythonUtils.java:
##########
@@ -37,35 +36,22 @@ public class PythonUtils {
   public static GatewayServer createGatewayServer(Object entryPoint,
                                                   String serverAddress,
                                                   int port,
-                                                  String secretKey,
-                                                  boolean useAuth) throws IOException {
-    LOGGER.info("Launching GatewayServer at " + serverAddress + ":" + port +
-        ", useAuth: " + useAuth);
-    if (useAuth) {
-      try {
-        Class clz = Class.forName("py4j.GatewayServer$GatewayServerBuilder", true,
-            Thread.currentThread().getContextClassLoader());
-        Object builder = clz.getConstructor(Object.class).newInstance(entryPoint);
-        builder.getClass().getMethod("authToken", String.class).invoke(builder, secretKey);
-        builder.getClass().getMethod("javaPort", int.class).invoke(builder, port);
-        builder.getClass().getMethod("javaAddress", InetAddress.class).invoke(builder,
-            InetAddress.getByName(serverAddress));
-        builder.getClass()
-            .getMethod("callbackClient", int.class, InetAddress.class, String.class)
-            .invoke(builder, port, InetAddress.getByName(serverAddress), secretKey);
-        return (GatewayServer) builder.getClass().getMethod("build").invoke(builder);
-      } catch (Exception e) {
-        throw new IOException(e);
-      }
-    } else {
-      return new GatewayServer(entryPoint,
-          port,
-          GatewayServer.DEFAULT_PYTHON_PORT,
-          InetAddress.getByName(serverAddress),
-          InetAddress.getByName(serverAddress),
-          GatewayServer.DEFAULT_CONNECT_TIMEOUT,
-          GatewayServer.DEFAULT_READ_TIMEOUT,
-          (List) null);
+                                                  String secretKey) throws IOException {
+    LOGGER.info("Launching GatewayServer at {}:{}", serverAddress, port);
+    try {
+      Class clz = Class.forName("py4j.GatewayServer$GatewayServerBuilder", true,

Review Comment:
   After eliminating the reflection, CI result is good



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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#discussion_r1350391386


##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncher.java:
##########
@@ -294,37 +292,17 @@ private String detectSparkScalaVersion(String sparkHome, Map<String, String> env
   }
 
   private String detectSparkScalaVersionByReplClass(String sparkHome) throws Exception {
-    File sparkLibFolder = new File(sparkHome + "/lib");
-    if (sparkLibFolder.exists()) {
-      // spark 1.6 if spark/lib exists
-      File[] sparkAssemblyJars = new File(sparkHome + "/lib").listFiles(new FilenameFilter() {
-        @Override
-        public boolean accept(File dir, String name) {
-          return name.contains("spark-assembly");
-        }
-      });
-      if (sparkAssemblyJars.length == 0) {
-        throw new Exception("No spark assembly file found in SPARK_HOME: " + sparkHome);
-      }
-      if (sparkAssemblyJars.length > 1) {
-        throw new Exception("Multiple spark assembly file found in SPARK_HOME: " + sparkHome);
-      }
-      try (URLClassLoader urlClassLoader = new URLClassLoader(new URL[]{sparkAssemblyJars[0].toURI().toURL()});){
-        urlClassLoader.loadClass("org.apache.spark.repl.SparkCommandLine");
-        return "2.10";
-      } catch (ClassNotFoundException e) {
-        return "2.11";
-      }
+    File sparkJarsFolder = new File(sparkHome + "/jars");
+    boolean sparkRepl212Exists =
+            Stream.of(sparkJarsFolder.listFiles()).anyMatch(file -> file.getName().contains("spark-repl_2.12"));
+    boolean sparkRepl213Exists =
+            Stream.of(sparkJarsFolder.listFiles()).anyMatch(file -> file.getName().contains("spark-repl_2.13"));
+    if (sparkRepl212Exists) {

Review Comment:
   I think it's fine to suppose that the user provides a valid SPARK_HOME, otherwise, things could be much more complicated. And, a valid Spark installation won't contain both `spark-repl_2.12` and `spark-repl_2.13`



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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#discussion_r1349748003


##########
python/src/main/java/org/apache/zeppelin/python/PythonInterpreter.java:
##########
@@ -116,7 +115,6 @@ public void open() throws InterpreterException {
     }
 
     try {
-      this.usePy4jAuth = Boolean.parseBoolean(getProperty("zeppelin.py4j.useAuth", "true"));

Review Comment:
   This change is based on the fact that `isSecretSupported` is always true for Spark 3.2 and above.
   
   property `zeppelin.py4j.useAuth` is only used internally, and is not exposed to the end user, the removal should be safe.



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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#discussion_r1350393297


##########
python/src/main/java/org/apache/zeppelin/python/PythonUtils.java:
##########
@@ -37,35 +36,22 @@ public class PythonUtils {
   public static GatewayServer createGatewayServer(Object entryPoint,
                                                   String serverAddress,
                                                   int port,
-                                                  String secretKey,
-                                                  boolean useAuth) throws IOException {
-    LOGGER.info("Launching GatewayServer at " + serverAddress + ":" + port +
-        ", useAuth: " + useAuth);
-    if (useAuth) {
-      try {
-        Class clz = Class.forName("py4j.GatewayServer$GatewayServerBuilder", true,
-            Thread.currentThread().getContextClassLoader());
-        Object builder = clz.getConstructor(Object.class).newInstance(entryPoint);
-        builder.getClass().getMethod("authToken", String.class).invoke(builder, secretKey);
-        builder.getClass().getMethod("javaPort", int.class).invoke(builder, port);
-        builder.getClass().getMethod("javaAddress", InetAddress.class).invoke(builder,
-            InetAddress.getByName(serverAddress));
-        builder.getClass()
-            .getMethod("callbackClient", int.class, InetAddress.class, String.class)
-            .invoke(builder, port, InetAddress.getByName(serverAddress), secretKey);
-        return (GatewayServer) builder.getClass().getMethod("build").invoke(builder);
-      } catch (Exception e) {
-        throw new IOException(e);
-      }
-    } else {
-      return new GatewayServer(entryPoint,
-          port,
-          GatewayServer.DEFAULT_PYTHON_PORT,
-          InetAddress.getByName(serverAddress),
-          InetAddress.getByName(serverAddress),
-          GatewayServer.DEFAULT_CONNECT_TIMEOUT,
-          GatewayServer.DEFAULT_READ_TIMEOUT,
-          (List) null);
+                                                  String secretKey) throws IOException {
+    LOGGER.info("Launching GatewayServer at {}:{}", serverAddress, port);
+    try {
+      Class clz = Class.forName("py4j.GatewayServer$GatewayServerBuilder", true,

Review Comment:
   I'm not very sure, it existed here without documents.
   
   Maybe we can eliminate the reflection now, let me try



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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#discussion_r1352296689


##########
zeppelin-zengine/src/main/java/org/apache/zeppelin/interpreter/launcher/SparkInterpreterLauncher.java:
##########
@@ -294,37 +292,17 @@ private String detectSparkScalaVersion(String sparkHome, Map<String, String> env
   }
 
   private String detectSparkScalaVersionByReplClass(String sparkHome) throws Exception {
-    File sparkLibFolder = new File(sparkHome + "/lib");
-    if (sparkLibFolder.exists()) {
-      // spark 1.6 if spark/lib exists
-      File[] sparkAssemblyJars = new File(sparkHome + "/lib").listFiles(new FilenameFilter() {
-        @Override
-        public boolean accept(File dir, String name) {
-          return name.contains("spark-assembly");
-        }
-      });
-      if (sparkAssemblyJars.length == 0) {
-        throw new Exception("No spark assembly file found in SPARK_HOME: " + sparkHome);
-      }
-      if (sparkAssemblyJars.length > 1) {
-        throw new Exception("Multiple spark assembly file found in SPARK_HOME: " + sparkHome);
-      }
-      try (URLClassLoader urlClassLoader = new URLClassLoader(new URL[]{sparkAssemblyJars[0].toURI().toURL()});){
-        urlClassLoader.loadClass("org.apache.spark.repl.SparkCommandLine");
-        return "2.10";
-      } catch (ClassNotFoundException e) {
-        return "2.11";
-      }
+    File sparkJarsFolder = new File(sparkHome + "/jars");
+    boolean sparkRepl212Exists =
+            Stream.of(sparkJarsFolder.listFiles()).anyMatch(file -> file.getName().contains("spark-repl_2.12"));
+    boolean sparkRepl213Exists =
+            Stream.of(sparkJarsFolder.listFiles()).anyMatch(file -> file.getName().contains("spark-repl_2.13"));
+    if (sparkRepl212Exists) {

Review Comment:
   OK, let me add such check too



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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [ZEPPELIN-5946][FOLLOWUP] Clean up code for legacy Spark versions [zeppelin]

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4668:
URL: https://github.com/apache/zeppelin/pull/4668#discussion_r1352293951


##########
python/src/main/java/org/apache/zeppelin/python/PythonUtils.java:
##########
@@ -37,35 +36,22 @@ public class PythonUtils {
   public static GatewayServer createGatewayServer(Object entryPoint,
                                                   String serverAddress,
                                                   int port,
-                                                  String secretKey,
-                                                  boolean useAuth) throws IOException {
-    LOGGER.info("Launching GatewayServer at " + serverAddress + ":" + port +
-        ", useAuth: " + useAuth);
-    if (useAuth) {
-      try {
-        Class clz = Class.forName("py4j.GatewayServer$GatewayServerBuilder", true,
-            Thread.currentThread().getContextClassLoader());
-        Object builder = clz.getConstructor(Object.class).newInstance(entryPoint);
-        builder.getClass().getMethod("authToken", String.class).invoke(builder, secretKey);
-        builder.getClass().getMethod("javaPort", int.class).invoke(builder, port);
-        builder.getClass().getMethod("javaAddress", InetAddress.class).invoke(builder,
-            InetAddress.getByName(serverAddress));
-        builder.getClass()
-            .getMethod("callbackClient", int.class, InetAddress.class, String.class)
-            .invoke(builder, port, InetAddress.getByName(serverAddress), secretKey);
-        return (GatewayServer) builder.getClass().getMethod("build").invoke(builder);
-      } catch (Exception e) {
-        throw new IOException(e);
-      }
-    } else {
-      return new GatewayServer(entryPoint,
-          port,
-          GatewayServer.DEFAULT_PYTHON_PORT,
-          InetAddress.getByName(serverAddress),
-          InetAddress.getByName(serverAddress),
-          GatewayServer.DEFAULT_CONNECT_TIMEOUT,
-          GatewayServer.DEFAULT_READ_TIMEOUT,
-          (List) null);
+                                                  String secretKey) throws IOException {
+    LOGGER.info("Launching GatewayServer at {}:{}", serverAddress, port);
+    try {
+      Class clz = Class.forName("py4j.GatewayServer$GatewayServerBuilder", true,

Review Comment:
   Great.



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

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org