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/19 10:05:52 UTC

[GitHub] [flink] aljoscha commented on a change in pull request #13554: [FLINK-19493] In CliFrontend, make flow of Configuration more obvious

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