From 5988d06fd5bc8e6a20e924bb009aaf3f9b5f884b Mon Sep 17 00:00:00 2001 From: Till Rohrmann Date: Mon, 15 May 2017 12:04:07 +0200 Subject: [PATCH 1/2] [FLINK-6581] [cli] Correct dynamic property parsing for YARN cli The YARN cli will now split the dynamic propertie at the first occurrence of the = sign instead of splitting it at every = sign. That way we support dynamic properties of the form -yDenv.java.opts="-DappName=foobar". --- .../apache/flink/yarn/FlinkYarnSessionCliTest.java | 6 ++++-- .../apache/flink/yarn/cli/FlinkYarnSessionCli.java | 12 +++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/flink-yarn-tests/src/test/java/org/apache/flink/yarn/FlinkYarnSessionCliTest.java b/flink-yarn-tests/src/test/java/org/apache/flink/yarn/FlinkYarnSessionCliTest.java index 264b6aaafc2b0..4da5a39eaa412 100644 --- a/flink-yarn-tests/src/test/java/org/apache/flink/yarn/FlinkYarnSessionCliTest.java +++ b/flink-yarn-tests/src/test/java/org/apache/flink/yarn/FlinkYarnSessionCliTest.java @@ -68,7 +68,8 @@ public void testDynamicProperties() throws IOException { CommandLineParser parser = new DefaultParser(); CommandLine cmd = null; try { - cmd = parser.parse(options, new String[]{"run", "-j", "fake.jar", "-n", "15", "-D", "akka.ask.timeout=5 min"}); + cmd = parser.parse(options, new String[]{"run", "-j", "fake.jar", "-n", "15", + "-D", "akka.ask.timeout=5 min", "-D", "env.java.opts=-DappName=foobar"}); } catch(Exception e) { e.printStackTrace(); Assert.fail("Parsing failed with " + e.getMessage()); @@ -80,8 +81,9 @@ public void testDynamicProperties() throws IOException { Map dynProperties = FlinkYarnSessionCli.getDynamicProperties(flinkYarnDescriptor.getDynamicPropertiesEncoded()); - Assert.assertEquals(1, dynProperties.size()); + Assert.assertEquals(2, dynProperties.size()); Assert.assertEquals("5 min", dynProperties.get("akka.ask.timeout")); + Assert.assertEquals("-DappName=foobar", dynProperties.get("env.java.opts")); } @Test diff --git a/flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java b/flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java index 3d82132f5f958..a291f18aafe1d 100644 --- a/flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java +++ b/flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java @@ -702,9 +702,15 @@ public static Map getDynamicProperties(String dynamicPropertiesE continue; } - String[] kv = propLine.split("="); - if (kv.length >= 2 && kv[0] != null && kv[1] != null && kv[0].length() > 0) { - properties.put(kv[0], kv[1]); + int firstEquals = propLine.indexOf("="); + + if (firstEquals >= 0) { + String key = propLine.substring(0, firstEquals).trim(); + String value = propLine.substring(firstEquals + 1, propLine.length()).trim(); + + if (!key.isEmpty() && !value.isEmpty()) { + properties.put(key, value); + } } } return properties; From 46d4318dfea8345a9b2e383f3ab5b08e133241d1 Mon Sep 17 00:00:00 2001 From: Till Rohrmann Date: Mon, 15 May 2017 15:50:48 +0200 Subject: [PATCH 2/2] Address PR comments --- .../java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java b/flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java index a291f18aafe1d..69b472aaf6308 100644 --- a/flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java +++ b/flink-yarn/src/main/java/org/apache/flink/yarn/cli/FlinkYarnSessionCli.java @@ -708,7 +708,7 @@ public static Map getDynamicProperties(String dynamicPropertiesE String key = propLine.substring(0, firstEquals).trim(); String value = propLine.substring(firstEquals + 1, propLine.length()).trim(); - if (!key.isEmpty() && !value.isEmpty()) { + if (!key.isEmpty()) { properties.put(key, value); } }