paul-rogers commented on code in PR #12512:
URL: https://github.com/apache/druid/pull/12512#discussion_r887376146
##########
extensions-core/kinesis-indexing-service/src/test/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisorTest.java:
##########
@@ -3911,8 +3907,87 @@ public void testIsTaskCurrent()
ImmutableSet.of()
);
+ return supervisor;
+ }
+
+ @Test
+ public void testIsTaskCurrentTaskFromStorage()
+ {
+ DateTime minMessageTime = DateTimes.nowUtc();
+ DateTime maxMessageTime = DateTimes.nowUtc().plus(10000);
+
+ KinesisSupervisor supervisor = isCurrentSetup(minMessageTime,
maxMessageTime);
+
+ KinesisIndexTask taskFromStorage = createKinesisIndexTask(
+ "id1",
+ 0,
+ new SeekableStreamStartSequenceNumbers<>("stream", ImmutableMap.of(
+ SHARD_ID1,
+ "3"
+ ), ImmutableSet.of()),
+ new SeekableStreamEndSequenceNumbers<>("stream", ImmutableMap.of(
+ SHARD_ID1,
+ KinesisSequenceNumber.NO_END_SEQUENCE_NUMBER
+ )),
+ minMessageTime,
+ maxMessageTime,
+ dataSchema
+ );
Review Comment:
Nice improvement in general. A drawback of this change is the copy/past of
the same block of lines. Suggestion, pull the material on lines 3916-3935 into
a method, taking the id as a parameter. You need two values back: `supervisor`
and `taskStorage`. This can be done using Druid's `Pair` class, or by creating
an ad-hoc class. The ad-hoc class might be nice: the setup could be done in the
constructor. Pick whatever solution you prefer, but let's try to avoid the
redundancy.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
