[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/664#discussion_r30391422
--- Diff: docs/apis/best_practices.md ---
@@ -0,0 +1,155 @@
+---
+title: Best Practices
+---
+!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+--
+
+a href=#top/a
+
+
+This page contains a collection of best practices for Flink programmers on
how to solve frequently encountered problems.
+
+
+* This will be replaced by the TOC
+{:toc}
+
+## Parsing command line arguments and passing them around in your Flink
application
+
+
+Almost all Flink applications, both batch and streaming rely on external
configuration parameters.
+For example for specifying input and output sources (like paths or
addresses), also system parameters (parallelism, runtime configuration) and
application specific parameters (often used within the user functions).
+
+Since version 0.9 we are providing a simple called `ParameterTool` to
provide at least some basic tooling for solving these problems.
--- End diff --
a simple ... = utility?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/664#discussion_r30392459
--- Diff: docs/apis/best_practices.md ---
@@ -0,0 +1,155 @@
+---
+title: Best Practices
+---
+!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+--
+
+a href=#top/a
+
+
+This page contains a collection of best practices for Flink programmers on
how to solve frequently encountered problems.
+
+
+* This will be replaced by the TOC
+{:toc}
+
+## Parsing command line arguments and passing them around in your Flink
application
+
+
+Almost all Flink applications, both batch and streaming rely on external
configuration parameters.
+For example for specifying input and output sources (like paths or
addresses), also system parameters (parallelism, runtime configuration) and
application specific parameters (often used within the user functions).
+
+Since version 0.9 we are providing a simple called `ParameterTool` to
provide at least some basic tooling for solving these problems.
+
+As you'll see Flink is very flexible when it comes to parsing input
parameters. You are free to choose any other framework, like [Commons
CLI](https://commons.apache.org/proper/commons-cli/),
[argparse4j](http://argparse4j.sourceforge.net/), or others.
--- End diff --
I rephrased it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user uce commented on the pull request: https://github.com/apache/flink/pull/664#issuecomment-102312636 Thanks for the docs update! Very nice idea with the best practices page. +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/664#discussion_r30392111 --- Diff: docs/apis/programming_guide.md --- @@ -665,6 +667,18 @@ DataSetInteger result = in.partitionByHash(0) /td /tr tr + tdstrongCustom Partitioning/strong/td + td --- End diff -- Yes, its a fix to https://issues.apache.org/jira/browse/FLINK-1260 ;) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/664#discussion_r30391587
--- Diff: docs/apis/best_practices.md ---
@@ -0,0 +1,155 @@
+---
+title: Best Practices
+---
+!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+--
+
+a href=#top/a
+
+
+This page contains a collection of best practices for Flink programmers on
how to solve frequently encountered problems.
+
+
+* This will be replaced by the TOC
+{:toc}
+
+## Parsing command line arguments and passing them around in your Flink
application
+
+
+Almost all Flink applications, both batch and streaming rely on external
configuration parameters.
+For example for specifying input and output sources (like paths or
addresses), also system parameters (parallelism, runtime configuration) and
application specific parameters (often used within the user functions).
+
+Since version 0.9 we are providing a simple called `ParameterTool` to
provide at least some basic tooling for solving these problems.
+
+As you'll see Flink is very flexible when it comes to parsing input
parameters. You are free to choose any other framework, like [Commons
CLI](https://commons.apache.org/proper/commons-cli/),
[argparse4j](http://argparse4j.sourceforge.net/), or others.
+
+
+### Getting your configuration values into the `ParameterTool`
+
+The `ParameterTool` provides a set of predefined static methods for
reading the configuration. The tool is internally expecting a `MapString,
String`, so its very easy to integrate it with your own configuration style.
+
+
+ From `.properties` files
+
+The following method will read a
[Properties](https://docs.oracle.com/javase/tutorial/essential/environment/properties.html)
file and provide the key/value pairs:
+{% highlight java %}
+String propertiesFile = /home/sam/flink/myjob.properties;
+ParameterTool parameter = ParameterTool.fromPropertiesFile(propertiesFile);
+{% endhighlight %}
+
+
+ From the command line arguments
+
+This allows getting arguments like `--input hdfs:///mydata --elements 42`
from the command line.
+{% highlight java %}
+public static void main(String[] args) {
+ ParameterTool parameter = ParameterTool.fromArgs(args);
+ // .. regular code ..
+{% endhighlight %}
+
+
+ From system properties
+
+When starting a JVM, you can pass system properties to it:
`-Dinput=hdfs:///mydata`. You can also initialize the `ParameterTool` from
these system properties:
+
+{% highlight java %}
+ParameterTool parameter = ParameterTool.fromSystemProperties();
+{% endhighlight %}
+
+
+### Using the parameters in your Flink program
+
+Now that we've got the parameters from somewhere (see above) we can use
them in various ways.
+
+**Directly from the `ParameterTool`**
+
+The `ParameterTool` itself has methods for accessing the values.
+{% highlight java %}
+ParameterTool parameters = // ...
+parameter.getRequired(input);
+parameter.get(output, myDefaultValue);
+parameter.getLong(expectedCount, -1L);
+parameter.getNumberOfParameters()
+// .. there are more methods available.
+{% endhighlight %}
+
+You can use the return values of these methods directly in the main()
method (=the client submitting the application).
+For example you could set the parallelism of a operator like this:
+
+{% highlight java %}
+ParameterTool parameters = ParameterTool.fromArgs(args);
+DataSetTuple2String, Integer counts = text.flatMap(new
Tokenizer()).setParallelism(parameters.getInt(mapParallelism, 2));
--- End diff --
Maybe to make this patter readable... do
```
int parallelism = parameters.get(mapParallelism, 2);
counts = setParalellism(parallelism);
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/664#issuecomment-102315511 Thanks for the review Henry and Ufuk! I addressed your concerns and updated the PR. I rebased the PR to the current master and filed two JIRAs for missing features https://issues.apache.org/jira/browse/FLINK-2018 https://issues.apache.org/jira/browse/FLINK-2017 I'll probably merge this change in the next 12 hours ... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user rmetzger commented on a diff in the pull request:
https://github.com/apache/flink/pull/664#discussion_r30391280
--- Diff:
flink-java/src/test/java/org/apache/flink/api/java/utils/ParameterToolTest.java
---
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.utils;
+
+import org.apache.flink.api.java.ClosureCleaner;
+import org.apache.flink.configuration.Configuration;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.Map;
+import java.util.Properties;
+
+public class ParameterToolTest {
+
+ @Rule
+ public TemporaryFolder tmp = new TemporaryFolder();
+
+
+ // - Parser tests -
+
+ @Test(expected = RuntimeException.class)
+ public void testIllegalArgs() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{berlin});
+ Assert.assertEquals(0, parameter.getNumberOfParameters());
+ }
+
+ @Test
+ public void testNoVal() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{-berlin});
+ Assert.assertEquals(1, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(berlin));
+ }
+
+ @Test
+ public void testNoValDouble() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{--berlin});
+ Assert.assertEquals(1, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(berlin));
+ }
+
+ @Test
+ public void testMultipleNoVal() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{--a, --b, --c, --d, --e, --f});
+ Assert.assertEquals(6, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(a));
+ Assert.assertTrue(parameter.has(b));
+ Assert.assertTrue(parameter.has(c));
+ Assert.assertTrue(parameter.has(d));
+ Assert.assertTrue(parameter.has(e));
+ Assert.assertTrue(parameter.has(f));
+ }
+
+ @Test
+ public void testMultipleNoValMixed() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{--a, -b, -c, -d, --e, --f});
+ Assert.assertEquals(6, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(a));
+ Assert.assertTrue(parameter.has(b));
+ Assert.assertTrue(parameter.has(c));
+ Assert.assertTrue(parameter.has(d));
+ Assert.assertTrue(parameter.has(e));
+ Assert.assertTrue(parameter.has(f));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testEmptyVal() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{--a, -b, --});
+ Assert.assertEquals(2, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(a));
+ Assert.assertTrue(parameter.has(b));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testEmptyValShort() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{--a, -b, -});
+ Assert.assertEquals(2, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(a));
+ Assert.assertTrue(parameter.has(b));
+ }
+
+
+
+ /*@Test
--- End diff --
I'll remove it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user uce commented on a diff in the pull request:
https://github.com/apache/flink/pull/664#discussion_r30391850
--- Diff: docs/apis/best_practices.md ---
@@ -0,0 +1,155 @@
+---
+title: Best Practices
+---
+!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+License); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+AS IS BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+--
+
+a href=#top/a
+
+
+This page contains a collection of best practices for Flink programmers on
how to solve frequently encountered problems.
+
+
+* This will be replaced by the TOC
+{:toc}
+
+## Parsing command line arguments and passing them around in your Flink
application
+
+
+Almost all Flink applications, both batch and streaming rely on external
configuration parameters.
+For example for specifying input and output sources (like paths or
addresses), also system parameters (parallelism, runtime configuration) and
application specific parameters (often used within the user functions).
+
+Since version 0.9 we are providing a simple called `ParameterTool` to
provide at least some basic tooling for solving these problems.
+
+As you'll see Flink is very flexible when it comes to parsing input
parameters. You are free to choose any other framework, like [Commons
CLI](https://commons.apache.org/proper/commons-cli/),
[argparse4j](http://argparse4j.sourceforge.net/), or others.
--- End diff --
I think this paragraph is confusing. I would remove it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user rmetzger commented on a diff in the pull request: https://github.com/apache/flink/pull/664#discussion_r30392280 --- Diff: flink-runtime/src/main/resources/log4j.properties --- @@ -18,7 +18,7 @@ # Convenience file for local debugging of the JobManager/TaskManager. -log4j.rootLogger=OFF, console +log4j.rootLogger=INFO, console --- End diff -- I'll very soon fix that issue so that we don't have to change the logging level every time we want to debug something. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user aljoscha commented on the pull request: https://github.com/apache/flink/pull/664#issuecomment-102327285 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user uce commented on a diff in the pull request: https://github.com/apache/flink/pull/664#discussion_r30391338 --- Diff: flink-runtime/src/main/resources/log4j.properties --- @@ -18,7 +18,7 @@ # Convenience file for local debugging of the JobManager/TaskManager. -log4j.rootLogger=OFF, console +log4j.rootLogger=INFO, console --- End diff -- back to OFF? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user uce commented on a diff in the pull request: https://github.com/apache/flink/pull/664#discussion_r30391736 --- Diff: docs/apis/programming_guide.md --- @@ -665,6 +667,18 @@ DataSetInteger result = in.partitionByHash(0) /td /tr tr + tdstrongCustom Partitioning/strong/td + td --- End diff -- This is unrelated to the change, right? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/664#issuecomment-102383548 Okay .. merging --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/664 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user hsaputra commented on a diff in the pull request:
https://github.com/apache/flink/pull/664#discussion_r30342062
--- Diff:
flink-java/src/test/java/org/apache/flink/api/java/utils/ParameterToolTest.java
---
@@ -0,0 +1,220 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.api.java.utils;
+
+import org.apache.flink.api.java.ClosureCleaner;
+import org.apache.flink.configuration.Configuration;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.Map;
+import java.util.Properties;
+
+public class ParameterToolTest {
+
+ @Rule
+ public TemporaryFolder tmp = new TemporaryFolder();
+
+
+ // - Parser tests -
+
+ @Test(expected = RuntimeException.class)
+ public void testIllegalArgs() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{berlin});
+ Assert.assertEquals(0, parameter.getNumberOfParameters());
+ }
+
+ @Test
+ public void testNoVal() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{-berlin});
+ Assert.assertEquals(1, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(berlin));
+ }
+
+ @Test
+ public void testNoValDouble() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{--berlin});
+ Assert.assertEquals(1, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(berlin));
+ }
+
+ @Test
+ public void testMultipleNoVal() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{--a, --b, --c, --d, --e, --f});
+ Assert.assertEquals(6, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(a));
+ Assert.assertTrue(parameter.has(b));
+ Assert.assertTrue(parameter.has(c));
+ Assert.assertTrue(parameter.has(d));
+ Assert.assertTrue(parameter.has(e));
+ Assert.assertTrue(parameter.has(f));
+ }
+
+ @Test
+ public void testMultipleNoValMixed() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{--a, -b, -c, -d, --e, --f});
+ Assert.assertEquals(6, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(a));
+ Assert.assertTrue(parameter.has(b));
+ Assert.assertTrue(parameter.has(c));
+ Assert.assertTrue(parameter.has(d));
+ Assert.assertTrue(parameter.has(e));
+ Assert.assertTrue(parameter.has(f));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testEmptyVal() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{--a, -b, --});
+ Assert.assertEquals(2, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(a));
+ Assert.assertTrue(parameter.has(b));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testEmptyValShort() {
+ ParameterTool parameter = ParameterTool.fromArgs(new
String[]{--a, -b, -});
+ Assert.assertEquals(2, parameter.getNumberOfParameters());
+ Assert.assertTrue(parameter.has(a));
+ Assert.assertTrue(parameter.has(b));
+ }
+
+
+
+ /*@Test
--- End diff --
Do you want to keep this test as optional? If yes then it is better to have
comments on why and when to uncomment this test. Otherwise, let's just remove
it for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user hsaputra commented on the pull request: https://github.com/apache/flink/pull/664#issuecomment-102111410 Hi @rmetzger, just did a pass and other than comments about unused test and broken Travis due to check style I think this PR is ready to go. +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user mxm commented on the pull request: https://github.com/apache/flink/pull/664#issuecomment-101579496 Do you want to allow empty keys like in `-- value` or even `--`? I think the parser should throw an exception in this case. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] flink pull request: [FLINK-1525]Introduction of a small input para...
Github user rmetzger commented on the pull request: https://github.com/apache/flink/pull/664#issuecomment-101303549 Yes. But my PR will provide two standard implementations (ParameterTool, Configuration), so only if users want more features they need to implement the config. Users have to cast it in their function, yes. @hsaputra: I renamed the class to `ParameterTool`. -- I think the pull request is in a mergeable state now. I didn't change the examples yet. Thats going to be a big change which I would like to do separately, once this is merged. Please review the PR ... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
