You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@oozie.apache.org by an...@apache.org on 2018/08/22 15:52:05 UTC

oozie git commit: OOZIE-3330 [spark-action] Remove double quotes inside plain option values (asalamon74 via andras.piros)

Repository: oozie
Updated Branches:
  refs/heads/master b6452cfb0 -> 1c1225f8b


OOZIE-3330 [spark-action] Remove double quotes inside plain option values (asalamon74 via andras.piros)


Project: http://git-wip-us.apache.org/repos/asf/oozie/repo
Commit: http://git-wip-us.apache.org/repos/asf/oozie/commit/1c1225f8
Tree: http://git-wip-us.apache.org/repos/asf/oozie/tree/1c1225f8
Diff: http://git-wip-us.apache.org/repos/asf/oozie/diff/1c1225f8

Branch: refs/heads/master
Commit: 1c1225f8b36c578d91ad70a301babdd652f85058
Parents: b6452cf
Author: Andras Piros <an...@cloudera.com>
Authored: Wed Aug 22 17:50:13 2018 +0200
Committer: Andras Piros <an...@cloudera.com>
Committed: Wed Aug 22 17:50:13 2018 +0200

----------------------------------------------------------------------
 release-log.txt                                 |  1 +
 .../action/hadoop/SparkOptionsSplitter.java     | 10 ++--
 .../action/hadoop/TestSparkArgsExtractor.java   | 50 +++++++++++++++++++-
 .../action/hadoop/TestSparkOptionsSplitter.java |  6 +++
 4 files changed, 62 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/oozie/blob/1c1225f8/release-log.txt
----------------------------------------------------------------------
diff --git a/release-log.txt b/release-log.txt
index c0db2fc..237ab06 100644
--- a/release-log.txt
+++ b/release-log.txt
@@ -1,5 +1,6 @@
 -- Oozie 5.1.0 release (trunk - unreleased)
 
+OOZIE-3330 [spark-action] Remove double quotes inside plain option values (asalamon74 via andras.piros)
 OOZIE-3329 [build] test-patch-30-distro improvement (asalamon74 via andras.piros)
 OOZIE-3324 Cannot compile with findbugs check (asalamon74 via pbacsko)
 OOZIE-3304 Parsing sharelib timestamps is not threadsafe (dionusos, matijhs via andras.piros)

http://git-wip-us.apache.org/repos/asf/oozie/blob/1c1225f8/sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java
----------------------------------------------------------------------
diff --git a/sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java b/sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java
index b614322..ac6ee81 100644
--- a/sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java
+++ b/sharelib/spark/src/main/java/org/apache/oozie/action/hadoop/SparkOptionsSplitter.java
@@ -18,6 +18,7 @@
 
 package org.apache.oozie.action.hadoop;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.commons.lang.StringUtils;
 
 import java.util.ArrayList;
@@ -49,7 +50,7 @@ class SparkOptionsSplitter {
      *     <li>{@code spark.executor.extraJavaOptions="-XX:HeapDumpPath=/tmp"}</li>
      * </ul>
      */
-    private static final String VALUE_HAS_QUOTES_AT_ENDS_REGEX = "[a-zA-Z0-9.]+=\".+\"";
+    private static final String VALUE_HAS_QUOTES_AT_ENDS_REGEX = "([a-zA-Z0-9.]+=)?\".+\"";
 
     /**
      * Matches an option key / value pair, where the value part has quotes in between.
@@ -65,7 +66,7 @@ class SparkOptionsSplitter {
      * </ul>
      */
     private static final String VALUE_HAS_QUOTES_IN_BETWEEN_REGEX =
-            "[a-zA-Z0-9.]+=.*(\\w\\s+\"\\w+[\\s+\\w]*\"|\"\\w+[\\s+\\w]*\"\\s+\\w)+.*";
+            "([a-zA-Z0-9.]+=)?.*(\\w\\s+\"\\w+[\\s+\\w]*\"|\"\\w+[\\s+\\w]*\"\\s+\\w)+.*";
 
     /**
      * Converts the options to be Spark-compatible.
@@ -133,9 +134,10 @@ class SparkOptionsSplitter {
      *     <li>{@code key="value1 value2 value3 value4"}: gets unquoted (has quotes both ends, and no quotes in between)</li>
      * </ul>
      * @param maybeEntirelyQuotedValue a {@code String} that is a parameter value but not necessarily quoted
-     * @return an unquoted version of the input {@String}, when {@code maybeEntirelyQuotedValue} had quotes at both ends, and didn't
-     * have any quotes in between. Else {@code maybeEntirelyQuotedValue}
+     * @return an unquoted version of the input {@code String}, when {@code maybeEntirelyQuotedValue} had quotes at both ends,
+     * and didn't have any quotes in between. Else {@code maybeEntirelyQuotedValue}
      */
+    @SuppressFBWarnings(value = {"REDOS"}, justification = "Complex regular expression")
     private static String unquoteEntirelyQuotedValue(final String maybeEntirelyQuotedValue) {
         final boolean hasQuotesAtEnds = maybeEntirelyQuotedValue.matches(VALUE_HAS_QUOTES_AT_ENDS_REGEX);
         final boolean hasQuotesInBetween = maybeEntirelyQuotedValue.matches(VALUE_HAS_QUOTES_IN_BETWEEN_REGEX);

http://git-wip-us.apache.org/repos/asf/oozie/blob/1c1225f8/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java
----------------------------------------------------------------------
diff --git a/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java b/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java
index 60ab8b9..7ccd26a 100644
--- a/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java
+++ b/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkArgsExtractor.java
@@ -225,7 +225,7 @@ public class TestSparkArgsExtractor {
     }
 
     @Test
-    public void testQuotedDriverAndExecutorExtraJavaOptionsParsing() throws Exception {
+    public void testQuotedConfDriverAndExecutorExtraJavaOptionsParsing() throws Exception {
         final Configuration actionConf = new Configuration();
         actionConf.set(SparkActionExecutor.SPARK_MASTER, "yarn");
         actionConf.set(SparkActionExecutor.SPARK_MODE, "client");
@@ -266,6 +266,54 @@ public class TestSparkArgsExtractor {
     }
 
     @Test
+    public void testQuotedPlainDriverJavaOptions() throws Exception {
+        final Configuration actionConf = new Configuration();
+        actionConf.set(SparkActionExecutor.SPARK_MASTER, "yarn");
+        actionConf.set(SparkActionExecutor.SPARK_MODE, "client");
+        actionConf.set(SparkActionExecutor.SPARK_CLASS, "org.apache.oozie.example.SparkFileCopy");
+        actionConf.set(SparkActionExecutor.SPARK_JOB_NAME, "Spark Copy File");
+        actionConf.set(SparkActionExecutor.SPARK_OPTS, "--queue queueName " +
+                "--driver-memory 1g " +
+                "--executor-memory 1g " +
+                "--num-executors 1 " +
+                "--executor-cores 1 " +
+                "--driver-java-options \"-DhostName=localhost -DuserName=userName -DpassWord=password -DportNum=portNum\"");
+        actionConf.set(SparkActionExecutor.SPARK_JAR, "/lib/test.jar");
+
+        final String[] mainArgs = {"arg0", "arg1"};
+        final List<String> sparkArgs = new SparkArgsExtractor(actionConf).extract(mainArgs);
+
+        assertEquals("Spark args mismatch",
+                Lists.newArrayList("--master", "yarn",
+                        "--deploy-mode", "client",
+                        "--name", "Spark Copy File",
+                        "--class", "org.apache.oozie.example.SparkFileCopy",
+                        "--queue", "queueName",
+                        "--driver-memory", "1g",
+                        "--executor-memory", "1g",
+                        "--num-executors", "1",
+                        "--executor-cores", "1",
+                        "--driver-java-options", "-DhostName=localhost -DuserName=userName -DpassWord=password -DportNum=portNum",
+                        "--conf", "spark.executor.extraClassPath=$PWD/*",
+                        "--conf", "spark.driver.extraClassPath=$PWD/*",
+                        "--conf", "spark.yarn.security.tokens.hadoopfs.enabled=false",
+                        "--conf", "spark.yarn.security.tokens.hive.enabled=false",
+                        "--conf", "spark.yarn.security.tokens.hbase.enabled=false",
+                        "--conf", "spark.yarn.security.credentials.hadoopfs.enabled=false",
+                        "--conf", "spark.yarn.security.credentials.hive.enabled=false",
+                        "--conf", "spark.yarn.security.credentials.hbase.enabled=false",
+                        "--conf", "spark.executor.extraJavaOptions=-Dlog4j.configuration=spark-log4j.properties",
+                        "--conf", "spark.driver.extraJavaOptions=-Dlog4j.configuration=spark-log4j.properties",
+                        "--files", "spark-log4j.properties,hive-site.xml",
+                        "--conf", "spark.yarn.jar=null",
+                        "--verbose",
+                        "/lib/test.jar",
+                        "arg0",
+                        "arg1"),
+                sparkArgs);
+    }
+
+    @Test
     public void testPropertiesFileMerging() throws Exception {
         final Configuration actionConf = new Configuration();
         actionConf.set(SparkActionExecutor.SPARK_MASTER, "yarn");

http://git-wip-us.apache.org/repos/asf/oozie/blob/1c1225f8/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java
----------------------------------------------------------------------
diff --git a/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java b/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java
index 4be9777..61ded5e 100644
--- a/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java
+++ b/sharelib/spark/src/test/java/org/apache/oozie/action/hadoop/TestSparkOptionsSplitter.java
@@ -38,6 +38,10 @@ public class TestSparkOptionsSplitter {
                         Arrays.asList("--option1", "value1")},
                 {"--option1   value1",
                         Arrays.asList("--option1", "value1")},
+                {"--option1 \"value1 value2\"",
+                        Arrays.asList("--option1", "value1 value2")},
+                {"--option1 \"value1 \"value2\" value3\"",
+                        Arrays.asList("--option1", "\"value1 \"value2\" value3\"")},
                 {"   --option1 value1   ",
                         Arrays.asList("--option1", "value1")},
                 {"--conf special=value1",
@@ -48,6 +52,8 @@ public class TestSparkOptionsSplitter {
                         Arrays.asList("--conf", "special=value1 value2")},
                 {" --conf special=\"value1 value2\"  ",
                         Arrays.asList("--conf", "special=value1 value2")},
+                {"--conf special=value1 \"value2\"",
+                        Arrays.asList("--conf", "special=value1 \"value2\"")},
                 {"--conf special=value1 value2 --conf value3",
                         Arrays.asList("--conf", "special=value1 value2", "--conf", "value3")},
                 {"--conf special1=value1 special2=\"value2 value3\" --conf value4",