You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/10/07 13:33:42 UTC
[GitHub] [flink] aljoscha opened a new pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
aljoscha opened a new pull request #13554:
URL: https://github.com/apache/flink/pull/13554
## What is the purpose of the change
Before, it was up to the `CustomCommandLine` implementation whether any `Configuration` was passed through from the flink-conf.yaml or wherever the base `Configuration` came from.
Now, we make the flow of the `Configuration` explicit in `CliFrontend.getEffectiveConfiguration()`. Instead of relying on the `Configuration` we get from the `CustomCommandLine` we ask the `CustomCommandLine` to materialize its settings and add them manually to an effective `Configuration` that the `CliFrontend` controls.
This removes the `Configuration` parameter from `CustomCommandLines` that don't need it anymore, such as `DefaultCLI`, which means we also have to touch tests. Also, the test for `DefaultCLI` is vastly simplified because the behaviour we were testing there is now in the new ITCase.
This adds a new integration test in `CliFrontendITCase` that verifies correct parameter passing and also verifies that command line arguments override base settings
I'm also addressing https://issues.apache.org/jira/browse/FLINK-19521 here because I noticed and filed that during implementation. It's a small change but should make the CLI more consistent.
## Verifying this change
- A new ITCase was added.
- Added test for dynamic properties on `DefaultCLI`
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? yes, dynamic properties for the DefaultCLI
- If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7c4621df4ac510fa1458cb6bbf34e2c8a127f42e Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265)
* ce307bc21cf9a8f654c187793498873e2855c903 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286)
* 41c47e76315dd2e2f8d1dd42647f0a8ca900a30e UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r501550609
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontend.java
##########
@@ -892,15 +895,20 @@ private JobID parseJobId(String jobIdString) throws CliArgsException {
* @throws FlinkException if something goes wrong
*/
private <ClusterID> void runClusterAction(CustomCommandLine activeCommandLine, CommandLine commandLine, ClusterAction<ClusterID> clusterAction) throws FlinkException {
- final Configuration executorConfig = activeCommandLine.applyCommandLineOptionsToConfiguration(commandLine);
- final ClusterClientFactory<ClusterID> clusterClientFactory = clusterClientServiceLoader.getClusterClientFactory(executorConfig);
+ final Configuration effectiveConfiguration = new Configuration(configuration);
+ final Configuration commandLineConfiguration = activeCommandLine.toConfiguration(commandLine);
+ effectiveConfiguration.addAll(commandLineConfiguration);
- final ClusterID clusterId = clusterClientFactory.getClusterId(executorConfig);
+ LOG.debug("Effective configuration: {}", effectiveConfiguration);
Review comment:
Given that the `env` itself can add config options later, maybe the message here could change to reflect that this is the `effective configuration at the CLI client` or `after the addition of the CLI options`. Although a bit esoteric, this is a debug message so I think it is ok. If we want the actual effective config I think we should go in the `env.executeAsync()`.
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontend.java
##########
@@ -1001,9 +1009,10 @@ public int parseParameters(String[] args) {
return handleParametrizationException(ppe);
} catch (ProgramMissingJobException pmje) {
return handleMissingJobException();
- } catch (Exception e) {
- return handleError(e);
}
+// catch (Exception e) {
Review comment:
Shouldn't this be either removed or uncommented?
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/GenericCLI.java
##########
@@ -109,27 +104,29 @@ public void addGeneralOptions(Options baseOptions) {
}
@Override
- public Configuration applyCommandLineOptionsToConfiguration(final CommandLine commandLine) {
- final Configuration effectiveConfiguration = new Configuration(baseConfiguration);
+ public Configuration toConfiguration(final CommandLine commandLine) {
+ final Configuration resultConfiguration = new Configuration();
+
Review comment:
Do we need this `if() {...}`? Eventually we will merge the configuration passed at the constructor to the one returned here, right? In addition, getting rid of accessing the outside configuration from here is one of the main points of this work, right?
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/DefaultCLI.java
##########
@@ -37,16 +38,24 @@
*/
public class DefaultCLI extends AbstractCustomCommandLine {
+ public static final String ID = "default";
+
private static final Option addressOption = new Option("m", "jobmanager", true,
"Address of the JobManager to which to connect. " +
"Use this flag to connect to a different JobManager than the one specified in the configuration. " +
"Attention: This option is respected only if the high-availability configuration is NONE.");
- public static final String ID = "default";
-
- public DefaultCLI(Configuration configuration) {
- super(configuration);
- }
+ /**
+ * Dynamic properties allow the user to specify additional configuration values with -D, such as
+ * <tt> -Dfs.overwrite-files=true -Dtaskmanager.memory.network.min=536346624</tt>.
+ */
Review comment:
If I am not mistaken, now all `CustomCommandLines` have dynamic options. So why not moving the option and the encode/decode logic to the base class or a utility class (for the encode/decode)?
Actually, I believe that this will bring the `Generic` and the `Default CLI` codebases even closer.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "a2fe3b72877befddfb6c8f954891ceaa32550332",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a2fe3b72877befddfb6c8f954891ceaa32550332",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ef47deeb469f6a6ad53287f787a23baacba3622b Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912)
* a2fe3b72877befddfb6c8f954891ceaa32550332 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dac393b4f92e4f150909fe862b97d02b3c3fa5f4 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-712652388
@flinkbot run azure
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-717309987
Thanks! I merged this.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7c4621df4ac510fa1458cb6bbf34e2c8a127f42e Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265)
* ce307bc21cf9a8f654c187793498873e2855c903 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286)
* 41c47e76315dd2e2f8d1dd42647f0a8ca900a30e Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289)
* dac393b4f92e4f150909fe862b97d02b3c3fa5f4 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-705391814
Ah, I forgot to mention this still has sth in it that I don't like: I change `CliFrontend.parseAndRun()` to bubble up an exception instead of swallowing it so that I can test it. Not sure if we want this, though.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r512050986
##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/cli/KubernetesSessionCli.java
##########
@@ -81,7 +81,8 @@ public KubernetesSessionCli(Configuration configuration, ClusterClientServiceLoa
public Configuration getEffectiveConfiguration(String[] args) throws CliArgsException {
Review comment:
👌
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "a2fe3b72877befddfb6c8f954891ceaa32550332",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8289",
"triggerID" : "a2fe3b72877befddfb6c8f954891ceaa32550332",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ef47deeb469f6a6ad53287f787a23baacba3622b Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912)
* a2fe3b72877befddfb6c8f954891ceaa32550332 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8289)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 41c47e76315dd2e2f8d1dd42647f0a8ca900a30e Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289)
* dac393b4f92e4f150909fe862b97d02b3c3fa5f4 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha closed pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha closed pull request #13554:
URL: https://github.com/apache/flink/pull/13554
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-705391814
Ah, I forgot to mention this still has sth in it that I don't like: I change `CliFrontend.parseAndRun()` to bubble up an exception instead of swallowing it so that I can test it. Not sure if we want this, though.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r512012032
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/GenericCLI.java
##########
@@ -105,44 +87,33 @@ public void addRunOptions(Options baseOptions) {
public void addGeneralOptions(Options baseOptions) {
baseOptions.addOption(executorOption);
baseOptions.addOption(targetOption);
- baseOptions.addOption(dynamicProperties);
+ baseOptions.addOption(DynamicPropertiesUtil.DYNAMIC_PROPERTIES);
}
@Override
- public Configuration applyCommandLineOptionsToConfiguration(final CommandLine commandLine) {
- final Configuration effectiveConfiguration = new Configuration(baseConfiguration);
+ public Configuration toConfiguration(final CommandLine commandLine) {
+ final Configuration resultConfiguration = new Configuration();
+
+ if (configuration.getOptional(DeploymentOptions.TARGET).isPresent()) {
Review comment:
Can you explain a bit why we do not remove this `if() {}` block? If it is in the base configuration, we will add it later, right?
##########
File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/cli/KubernetesSessionCli.java
##########
@@ -81,7 +81,8 @@ public KubernetesSessionCli(Configuration configuration, ClusterClientServiceLoa
public Configuration getEffectiveConfiguration(String[] args) throws CliArgsException {
Review comment:
This can now become package private and be annotated as `@VisibleForTesting` I think.
##########
File path: flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/local/ExecutionContext.java
##########
@@ -341,8 +341,7 @@ private static Configuration createExecutionConfig(
availableCommandLines,
activeCommandLine);
- Configuration executionConfig = activeCommandLine.applyCommandLineOptionsToConfiguration(
- commandLine);
+ Configuration executionConfig = activeCommandLine.toConfiguration(commandLine);
Review comment:
I am not sure if here now we are missing the options from the base configuration. I will check further but I am also writing it as a comment so that you can also have a look.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "",
"status" : "CANCELED",
"url" : "TBD",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* Unknown: [CANCELED](TBD)
* ef47deeb469f6a6ad53287f787a23baacba3622b UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* ce307bc21cf9a8f654c187793498873e2855c903 Azure: [CANCELED](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286)
* 41c47e76315dd2e2f8d1dd42647f0a8ca900a30e Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289)
* dac393b4f92e4f150909fe862b97d02b3c3fa5f4 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7c4621df4ac510fa1458cb6bbf34e2c8a127f42e Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265)
* ce307bc21cf9a8f654c187793498873e2855c903 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286)
* 41c47e76315dd2e2f8d1dd42647f0a8ca900a30e Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r512119691
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/GenericCLI.java
##########
@@ -105,44 +87,33 @@ public void addRunOptions(Options baseOptions) {
public void addGeneralOptions(Options baseOptions) {
baseOptions.addOption(executorOption);
baseOptions.addOption(targetOption);
- baseOptions.addOption(dynamicProperties);
+ baseOptions.addOption(DynamicPropertiesUtil.DYNAMIC_PROPERTIES);
}
@Override
- public Configuration applyCommandLineOptionsToConfiguration(final CommandLine commandLine) {
- final Configuration effectiveConfiguration = new Configuration(baseConfiguration);
+ public Configuration toConfiguration(final CommandLine commandLine) {
+ final Configuration resultConfiguration = new Configuration();
+
+ if (configuration.getOptional(DeploymentOptions.TARGET).isPresent()) {
Review comment:
Ahh yes, you're right. I wasn't thinking straight.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r507633257
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontend.java
##########
@@ -1001,9 +1009,10 @@ public int parseParameters(String[] args) {
return handleParametrizationException(ppe);
} catch (ProgramMissingJobException pmje) {
return handleMissingJobException();
- } catch (Exception e) {
- return handleError(e);
}
+// catch (Exception e) {
Review comment:
Yes, this is the leftover I mentioned in my first comment. I've reworked the test to instead check stdout printing.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "a2fe3b72877befddfb6c8f954891ceaa32550332",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8289",
"triggerID" : "a2fe3b72877befddfb6c8f954891ceaa32550332",
"triggerType" : "PUSH"
}, {
"hash" : "f4f309e1e8e3bdc02d02d4ac89d8b8afac635465",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "f4f309e1e8e3bdc02d02d4ac89d8b8afac635465",
"triggerType" : "PUSH"
}, {
"hash" : "41da5158b49c1a53ac687343482e1e7365ad8fc6",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "41da5158b49c1a53ac687343482e1e7365ad8fc6",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a2fe3b72877befddfb6c8f954891ceaa32550332 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8289)
* f4f309e1e8e3bdc02d02d4ac89d8b8afac635465 UNKNOWN
* 41da5158b49c1a53ac687343482e1e7365ad8fc6 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r507627981
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontend.java
##########
@@ -892,15 +895,20 @@ private JobID parseJobId(String jobIdString) throws CliArgsException {
* @throws FlinkException if something goes wrong
*/
private <ClusterID> void runClusterAction(CustomCommandLine activeCommandLine, CommandLine commandLine, ClusterAction<ClusterID> clusterAction) throws FlinkException {
- final Configuration executorConfig = activeCommandLine.applyCommandLineOptionsToConfiguration(commandLine);
- final ClusterClientFactory<ClusterID> clusterClientFactory = clusterClientServiceLoader.getClusterClientFactory(executorConfig);
+ final Configuration effectiveConfiguration = new Configuration(configuration);
+ final Configuration commandLineConfiguration = activeCommandLine.toConfiguration(commandLine);
+ effectiveConfiguration.addAll(commandLineConfiguration);
- final ClusterID clusterId = clusterClientFactory.getClusterId(executorConfig);
+ LOG.debug("Effective configuration: {}", effectiveConfiguration);
Review comment:
Will add that. 👌
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] kl0u commented on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
kl0u commented on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704990097
Will have a look in a bit.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
} ]
}-->
## CI report:
* ef47deeb469f6a6ad53287f787a23baacba3622b Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dac393b4f92e4f150909fe862b97d02b3c3fa5f4 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291)
* d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "a2fe3b72877befddfb6c8f954891ceaa32550332",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8289",
"triggerID" : "a2fe3b72877befddfb6c8f954891ceaa32550332",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a2fe3b72877befddfb6c8f954891ceaa32550332 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8289)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r507626412
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/GenericCLI.java
##########
@@ -109,27 +104,29 @@ public void addGeneralOptions(Options baseOptions) {
}
@Override
- public Configuration applyCommandLineOptionsToConfiguration(final CommandLine commandLine) {
- final Configuration effectiveConfiguration = new Configuration(baseConfiguration);
+ public Configuration toConfiguration(final CommandLine commandLine) {
+ final Configuration resultConfiguration = new Configuration();
+
Review comment:
I see your point. For now we still use the `Configuration` in `isActive()`. We could think about not keeping the `Configuration` in the CLI but instead pass it as a parameter there as well. Also, I'm forwarding the option for now to make the returned `Configuration` "self contained", i.e. to have everything in there that the executor would expect.
What do you think?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r512057043
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/GenericCLI.java
##########
@@ -105,44 +87,33 @@ public void addRunOptions(Options baseOptions) {
public void addGeneralOptions(Options baseOptions) {
baseOptions.addOption(executorOption);
baseOptions.addOption(targetOption);
- baseOptions.addOption(dynamicProperties);
+ baseOptions.addOption(DynamicPropertiesUtil.DYNAMIC_PROPERTIES);
}
@Override
- public Configuration applyCommandLineOptionsToConfiguration(final CommandLine commandLine) {
- final Configuration effectiveConfiguration = new Configuration(baseConfiguration);
+ public Configuration toConfiguration(final CommandLine commandLine) {
+ final Configuration resultConfiguration = new Configuration();
+
+ if (configuration.getOptional(DeploymentOptions.TARGET).isPresent()) {
Review comment:
I know, but I do not really get it. I am not suggesting to remove the configuration from the class. I am just suggesting to not put the `TARGET` in the `resultConfiguration` but simply put whatever the user has put in the command line. I think that this is more consistent with the contract and also how the other custom command lines work.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7c4621df4ac510fa1458cb6bbf34e2c8a127f42e Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265)
* ce307bc21cf9a8f654c187793498873e2855c903 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* dac393b4f92e4f150909fe862b97d02b3c3fa5f4 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291)
* d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7c4621df4ac510fa1458cb6bbf34e2c8a127f42e Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265)
* ce307bc21cf9a8f654c187793498873e2855c903 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286)
* 41c47e76315dd2e2f8d1dd42647f0a8ca900a30e Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289)
* dac393b4f92e4f150909fe862b97d02b3c3fa5f4 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704940215
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.
## Automated Checks
Last check on commit 7c4621df4ac510fa1458cb6bbf34e2c8a127f42e (Wed Oct 07 13:35:27 UTC 2020)
**Warnings:**
* No documentation files were touched! Remember to keep the Flink docs up to date!
<sub>Mention the bot in a comment to re-run the automated checks.</sub>
## Review Progress
* ❓ 1. The [description] looks good.
* ❓ 2. There is [consensus] that the contribution should go into to Flink.
* ❓ 3. Needs [attention] from.
* ❓ 4. The change fits into the overall [architecture].
* ❓ 5. Overall code [quality] is good.
Please see the [Pull Request Review Guide](https://flink.apache.org/contributing/reviewing-prs.html) for a full explanation of the review process.<details>
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required <summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot approve description` to approve one or more aspects (aspects: `description`, `consensus`, `architecture` and `quality`)
- `@flinkbot approve all` to approve all aspects
- `@flinkbot approve-until architecture` to approve everything until `architecture`
- `@flinkbot attention @username1 [@username2 ..]` to require somebody's attention
- `@flinkbot disapprove architecture` to remove an approval you gave earlier
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot commented on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot commented on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7c4621df4ac510fa1458cb6bbf34e2c8a127f42e UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "CANCELED",
"url" : "TBD",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* Unknown: [CANCELED](TBD)
* ef47deeb469f6a6ad53287f787a23baacba3622b Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] kl0u commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
kl0u commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r501550609
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontend.java
##########
@@ -892,15 +895,20 @@ private JobID parseJobId(String jobIdString) throws CliArgsException {
* @throws FlinkException if something goes wrong
*/
private <ClusterID> void runClusterAction(CustomCommandLine activeCommandLine, CommandLine commandLine, ClusterAction<ClusterID> clusterAction) throws FlinkException {
- final Configuration executorConfig = activeCommandLine.applyCommandLineOptionsToConfiguration(commandLine);
- final ClusterClientFactory<ClusterID> clusterClientFactory = clusterClientServiceLoader.getClusterClientFactory(executorConfig);
+ final Configuration effectiveConfiguration = new Configuration(configuration);
+ final Configuration commandLineConfiguration = activeCommandLine.toConfiguration(commandLine);
+ effectiveConfiguration.addAll(commandLineConfiguration);
- final ClusterID clusterId = clusterClientFactory.getClusterId(executorConfig);
+ LOG.debug("Effective configuration: {}", effectiveConfiguration);
Review comment:
Given that the `env` itself can add config options later, maybe the message here could change to reflect that this is the `effective configuration at the CLI client` or `after the addition of the CLI options`. Although a bit esoteric, this is a debug message so I think it is ok. If we want the actual effective config I think we should go in the `env.executeAsync()`.
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontend.java
##########
@@ -1001,9 +1009,10 @@ public int parseParameters(String[] args) {
return handleParametrizationException(ppe);
} catch (ProgramMissingJobException pmje) {
return handleMissingJobException();
- } catch (Exception e) {
- return handleError(e);
}
+// catch (Exception e) {
Review comment:
Shouldn't this be either removed or uncommented?
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/GenericCLI.java
##########
@@ -109,27 +104,29 @@ public void addGeneralOptions(Options baseOptions) {
}
@Override
- public Configuration applyCommandLineOptionsToConfiguration(final CommandLine commandLine) {
- final Configuration effectiveConfiguration = new Configuration(baseConfiguration);
+ public Configuration toConfiguration(final CommandLine commandLine) {
+ final Configuration resultConfiguration = new Configuration();
+
Review comment:
Do we need this `if() {...}`? Eventually we will merge the configuration passed at the constructor to the one returned here, right? In addition, getting rid of accessing the outside configuration from here is one of the main points of this work, right?
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/DefaultCLI.java
##########
@@ -37,16 +38,24 @@
*/
public class DefaultCLI extends AbstractCustomCommandLine {
+ public static final String ID = "default";
+
private static final Option addressOption = new Option("m", "jobmanager", true,
"Address of the JobManager to which to connect. " +
"Use this flag to connect to a different JobManager than the one specified in the configuration. " +
"Attention: This option is respected only if the high-availability configuration is NONE.");
- public static final String ID = "default";
-
- public DefaultCLI(Configuration configuration) {
- super(configuration);
- }
+ /**
+ * Dynamic properties allow the user to specify additional configuration values with -D, such as
+ * <tt> -Dfs.overwrite-files=true -Dtaskmanager.memory.network.min=536346624</tt>.
+ */
Review comment:
If I am not mistaken, now all `CustomCommandLines` have dynamic options. So why not moving the option and the encode/decode logic to the base class or a utility class (for the encode/decode)?
Actually, I believe that this will bring the `Generic` and the `Default CLI` codebases even closer.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r512056865
##########
File path: flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/local/ExecutionContext.java
##########
@@ -341,8 +341,7 @@ private static Configuration createExecutionConfig(
availableCommandLines,
activeCommandLine);
- Configuration executionConfig = activeCommandLine.applyCommandLineOptionsToConfiguration(
- commandLine);
+ Configuration executionConfig = activeCommandLine.toConfiguration(commandLine);
Review comment:
This works because someone was clever enough to think about this case before. 😅
https://github.com/apache/flink/blame/733c684f19bc9d6167703e977f44d817e3ec5e69/flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/local/ExecutionContext.java#L192
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7c4621df4ac510fa1458cb6bbf34e2c8a127f42e Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7c4621df4ac510fa1458cb6bbf34e2c8a127f42e Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 7c4621df4ac510fa1458cb6bbf34e2c8a127f42e Azure: [FAILURE](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265)
* ce307bc21cf9a8f654c187793498873e2855c903 Azure: [PENDING](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286)
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] flinkbot edited a comment on pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
flinkbot edited a comment on pull request #13554:
URL: https://github.com/apache/flink/pull/13554#issuecomment-704950126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7265",
"triggerID" : "7c4621df4ac510fa1458cb6bbf34e2c8a127f42e",
"triggerType" : "PUSH"
}, {
"hash" : "ce307bc21cf9a8f654c187793498873e2855c903",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7286",
"triggerID" : "ce307bc21cf9a8f654c187793498873e2855c903",
"triggerType" : "PUSH"
}, {
"hash" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7289",
"triggerID" : "41c47e76315dd2e2f8d1dd42647f0a8ca900a30e",
"triggerType" : "PUSH"
}, {
"hash" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7291",
"triggerID" : "dac393b4f92e4f150909fe862b97d02b3c3fa5f4",
"triggerType" : "PUSH"
}, {
"hash" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7859",
"triggerID" : "d48f093ac588e43cbac9b1cf1adc8a2cc0c66e0e",
"triggerType" : "PUSH"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=7912",
"triggerID" : "ef47deeb469f6a6ad53287f787a23baacba3622b",
"triggerType" : "PUSH"
}, {
"hash" : "",
"status" : "DELETED",
"url" : "TBD",
"triggerID" : "712652388",
"triggerType" : "MANUAL"
}, {
"hash" : "a2fe3b72877befddfb6c8f954891ceaa32550332",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8289",
"triggerID" : "a2fe3b72877befddfb6c8f954891ceaa32550332",
"triggerType" : "PUSH"
}, {
"hash" : "f4f309e1e8e3bdc02d02d4ac89d8b8afac635465",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "f4f309e1e8e3bdc02d02d4ac89d8b8afac635465",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a2fe3b72877befddfb6c8f954891ceaa32550332 Azure: [SUCCESS](https://dev.azure.com/apache-flink/98463496-1af2-4620-8eab-a2ecc1a2e6fe/_build/results?buildId=8289)
* f4f309e1e8e3bdc02d02d4ac89d8b8afac635465 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run travis` re-run the last Travis build
- `@flinkbot run azure` re-run the last Azure build
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r512052055
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/GenericCLI.java
##########
@@ -105,44 +87,33 @@ public void addRunOptions(Options baseOptions) {
public void addGeneralOptions(Options baseOptions) {
baseOptions.addOption(executorOption);
baseOptions.addOption(targetOption);
- baseOptions.addOption(dynamicProperties);
+ baseOptions.addOption(DynamicPropertiesUtil.DYNAMIC_PROPERTIES);
}
@Override
- public Configuration applyCommandLineOptionsToConfiguration(final CommandLine commandLine) {
- final Configuration effectiveConfiguration = new Configuration(baseConfiguration);
+ public Configuration toConfiguration(final CommandLine commandLine) {
+ final Configuration resultConfiguration = new Configuration();
+
+ if (configuration.getOptional(DeploymentOptions.TARGET).isPresent()) {
Review comment:
I posted this answer earlier (https://github.com/apache/flink/pull/13554#discussion_r507626412):
> I see your point. For now we still use the Configuration in isActive(). We could think about not keeping the Configuration in the CLI but instead pass it as a parameter there as well. Also, I'm forwarding the option for now to make the returned Configuration "self contained", i.e. to have everything in there that the executor would expect.
>
> What do you think?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [flink] aljoscha commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious
Posted by GitBox <gi...@apache.org>.
aljoscha commented on a change in pull request #13554:
URL: https://github.com/apache/flink/pull/13554#discussion_r507626849
##########
File path: flink-clients/src/main/java/org/apache/flink/client/cli/DefaultCLI.java
##########
@@ -37,16 +38,24 @@
*/
public class DefaultCLI extends AbstractCustomCommandLine {
+ public static final String ID = "default";
+
private static final Option addressOption = new Option("m", "jobmanager", true,
"Address of the JobManager to which to connect. " +
"Use this flag to connect to a different JobManager than the one specified in the configuration. " +
"Attention: This option is respected only if the high-availability configuration is NONE.");
- public static final String ID = "default";
-
- public DefaultCLI(Configuration configuration) {
- super(configuration);
- }
+ /**
+ * Dynamic properties allow the user to specify additional configuration values with -D, such as
+ * <tt> -Dfs.overwrite-files=true -Dtaskmanager.memory.network.min=536346624</tt>.
+ */
Review comment:
I'll move it to a Util. Unfortunately, the `GenericCLI` and `DefaultCLI` don't have a common base class. And I also don't want to touch the dynamic properties code of the `FlinkYarnSessionCli` which is yet another story... 😅
----------------------------------------------------------------
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:
users@infra.apache.org