azagrebin commented on a change in pull request #13958:
URL: https://github.com/apache/flink/pull/13958#discussion_r519879358
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/adapter/DefaultExecutionTopology.java
##########
@@ -51,20 +53,20 @@
private static final Logger LOG =
LoggerFactory.getLogger(DefaultExecutionTopology.class);
- private final ExecutionGraph executionGraph;
-
private final Map<ExecutionVertexID, DefaultExecutionVertex>
executionVerticesById;
private final List<DefaultExecutionVertex> executionVerticesList;
private final Map<IntermediateResultPartitionID,
DefaultResultPartition> resultPartitionsById;
- private final Map<ExecutionVertexID, DefaultSchedulingPipelinedRegion>
pipelinedRegionsByVertex;
+ @Nullable
+ private Map<ExecutionVertexID, DefaultSchedulingPipelinedRegion>
pipelinedRegionsByVertex;
- private final List<DefaultSchedulingPipelinedRegion> pipelinedRegions;
+ @Nullable
+ private List<DefaultSchedulingPipelinedRegion> pipelinedRegions;
- public DefaultExecutionTopology(ExecutionGraph graph) {
- this.executionGraph = checkNotNull(graph, "execution graph can
not be null");
+ private DefaultExecutionTopology(ExecutionGraph graph) {
Review comment:
I meant testing `DefaultExecutionTopology` itself and its construction
process separately
but it seems that tests have already been written for the constructor.
In this case, I would keep it as it was with final fields at least (it
provides immutability and better guarantees)
if you think that construction does not need to be separated from the
`DefaultExecutionTopology` as just a data holder or we can discuss it in
another ticket.
----------------------------------------------------------------
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:
us...@infra.apache.org
