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/03/27 19:12:34 UTC
[GitHub] [flink] mxm commented on a change in pull request #11473:
[FLINK-16705] Ensure MiniCluster shutdown does not interfere with JobResult
retrieval
mxm commented on a change in pull request #11473: [FLINK-16705] Ensure MiniCluster shutdown does not interfere with JobResult retrieval
URL: https://github.com/apache/flink/pull/11473#discussion_r399485207
##########
File path: flink-tests/src/test/java/org/apache/flink/test/example/client/LocalExecutorITCase.java
##########
@@ -81,4 +114,35 @@ private Plan getWordCountPlan(File inFile, File outFile, int parallelism) {
.writeAsCsv(outFile.getAbsolutePath());
return env.createProgramPlan();
}
+
+ private Plan getRuntimeExceptionPlan() {
+ ExecutionEnvironment env = ExecutionEnvironment.getExecutionEnvironment();
+ env.fromElements(1)
+ .map(element -> {
+ if (element == 1) {
+ throw new RuntimeException("oups");
+ }
+ return element;
+ })
+ .output(new DiscardingOutputFormat<>());
+ return env.createProgramPlan();
+ }
+
+ /**
+ * A normal {@link LocalExecutor} but with the option to retrieve the Minicluster.
+ */
+ private static class InspectableLocalExecutor extends LocalExecutor {
+
+ private MiniCluster miniCluster;
+
+ @Override
+ protected MiniCluster startMiniCluster(JobGraph jobGraph, Configuration configuration) throws Exception {
+ return miniCluster = super.startMiniCluster(jobGraph, configuration);
+ }
+
+ public boolean isMiniClusterRunning() {
+ Preconditions.checkNotNull(miniCluster, "MiniCluster has not been started yet.");
+ return miniCluster.isRunning();
+ }
+ }
Review comment:
IMHO that's a personal preference, either way we alter the main code to make it testable. Introducing additional abstractions for testing can make the code harder to read and maintain. If there is a factory for everything, it can be harder to keep track of what the code is doing.
Nevertheless, I think your suggestion makes sense. I've implemented a factory in the latest commit.
----------------------------------------------------------------
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