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:
[email protected]
