[GitHub] [flink] kl0u commented on a change in pull request #9607: [FLINK-13946] Remove job session related code from ExecutionEnvironment

[GitBox <gi...@apache.org>]

kl0u commented on a change in pull request #9607: [FLINK-13946] Remove job session related code from ExecutionEnvironment
URL: https://github.com/apache/flink/pull/9607#discussion_r320721408
 
 

 ##########
 File path: flink-java/src/main/java/org/apache/flink/api/java/LocalEnvironment.java
 ##########
 @@ -71,150 +63,34 @@ public LocalEnvironment(Configuration config) {
 					"The LocalEnvironment cannot be instantiated when running in a pre-defined context " +
 							"(such as Command Line Client, Scala Shell, or TestEnvironment)");
 		}
-		this.configuration = config == null ? new Configuration() : config;
+		this.configuration = checkNotNull(config);
 	}
 
 	// --------------------------------------------------------------------------------------------
 
+	// TODO: 31.08.19 make sure that start and stop are called in the execute.
+	// the other place would be here, but this can complicate code, as the
+	// lifecycle management would be outside the executor itself.
+
 	@Override
 	public JobExecutionResult execute(String jobName) throws Exception {
-		if (executor == null) {
-			startNewSession();
-		}
-
-		Plan p = createProgramPlan(jobName);
+		final Plan p = createProgramPlan(jobName);
 
-		// Session management is disabled, revert this commit to enable
-		//p.setJobId(jobID);
-		//p.setSessionTimeout(sessionTimeout);
-
-		JobExecutionResult result = executor.executePlan(p);
-
-		this.lastJobExecutionResult = result;
-		return result;
+		// TODO: 31.08.19 make the executor autocloseable
 
 Review comment:
   I agree on both points:
   1) We should unify the executors for both batch and streaming but this is a bigger issue and will be addressed later.
   2) It is not worth spending a lot of time in the `PlanExecutor`s, but at least let's make the code clearer.

----------------------------------------------------------------
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


With regards,
Apache Git Services