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