[GitHub] [flink] kl0u commented on a change in pull request #11671: [FLINK-17052] [client] Introduce PlanGenerator

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

kl0u commented on a change in pull request #11671: [FLINK-17052] [client] Introduce PlanGenerator
URL: https://github.com/apache/flink/pull/11671#discussion_r407972646
 
 

 ##########
 File path: flink-java/src/main/java/org/apache/flink/api/java/ExecutionEnvironment.java
 ##########
 @@ -1035,6 +1030,14 @@ protected void registerCachedFilesWithPlan(Plan p) throws IOException {
 		}
 	}
 
+	/**
 
 Review comment:
   From the discussion, it seems that this method was added:
   1) for the tests 
   2) for future usage in the `TableEnvironment`.
   
   If this is correct, I would suggest to remove it from this PR and add it to the PR that uses it. Now it seems that this is an unused `public` method.
   
   In addition, I agree with @aljoscha 's concerns that we may need to come up with a better and more foolproof design on how to forward stuff from the `ExecutionEnvironment` to the `TableEnvironment`. So if we achieve that, then this method would be instant legacy. 
   
   WDYT?

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