StephanEwen commented on a change in pull request #12507:
URL: https://github.com/apache/flink/pull/12507#discussion_r438741095
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/source/event/AddSplitEvent.java
##########
@@ -30,15 +33,23 @@ Licensed to the Apache Software Foundation (ASF) under one
public class AddSplitEvent<SplitT> implements OperatorEvent {
private static final long serialVersionUID = 1L;
-
- private final List<SplitT> splits;
-
- public AddSplitEvent(List<SplitT> splits) {
- this.splits = splits;
+ private final int serializerVersion;
+ private final List<byte[]> splits;
Review comment:
Nit: For serializable classes it is nice to put the concrete type of the
list there, because not all list implementations are serializable. And because
this is a purely internal field, typing this to `ArrayList` wouldn't change any
abstractions.
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/source/coordinator/SourceCoordinatorContextTest.java
##########
@@ -71,16 +72,16 @@ public void testUnregisterReader() {
}
@Test
- public void testAssignSplitsFromCoordinatorExecutor() throws
ExecutionException, InterruptedException {
+ public void testAssignSplitsFromCoordinatorExecutor() throws
ExecutionException, InterruptedException, IOException {
Review comment:
Nit: For tests it is often easier to just declare `throws Exception`,
rather than have a long list of individual exceptions.
----------------------------------------------------------------
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:
[email protected]
