Everything else #971
Everything else #971
Conversation
| @@ -64,24 +61,36 @@ class ShardConsumer { | |||
| private final long taskBackoffTimeMillis; | |||
| private final boolean skipShardSyncAtWorkerInitializationIfLeasesExist; | |||
|
|
|||
| @Getter | |||
| //@Getter | |||
There was a problem hiding this comment.
why are we commenting it out and adding separate get function? same for below two commented gets
There was a problem hiding this comment.
This is a carry over from the brazil testing as the build couldn't find the getter functions. I will verify that maven can properly read the getter, and remove these changes if tests pass.
| @@ -373,9 +382,9 @@ public boolean isSkipShardSyncAtWorkerInitializationIfLeasesExist() { | |||
| return skipShardSyncAtWorkerInitializationIfLeasesExist; | |||
| } | |||
|
|
|||
| private enum TaskOutcome { | |||
| /*public enum TaskOutcome { | |||
There was a problem hiding this comment.
Since you have moved this to IShardConsumer, why not just remove this?
There was a problem hiding this comment.
Just a carry over from testing. I will remove it.
| if(shardConsumerFactory == null){ | ||
|
|
||
| shardConsumerFactory = new KinesisShardConsumerFactory(); | ||
| } | ||
|
|
||
| return shardConsumerFactory.createShardConsumer(shardInfo, |
There was a problem hiding this comment.
Will there be any conflict if only shardConsumerFactory is set to Kinesis variant and all other are of different type?
There was a problem hiding this comment.
I am unsure what you mean by this. The chosen ShardConsumerFactory should create all ShardConsumers so they should all be the same type. Do you mean if the ShardConsumerFactory is the KinesisVariant but other classes like the PeriodicShardSyncManager are the DDB Streams Variant?
There was a problem hiding this comment.
Do you mean if the ShardConsumerFactory is the KinesisVariant but other classes like the PeriodicShardSyncManager are the DDB Streams Variant?
yes, will it cause any issue?
There was a problem hiding this comment.
So a normal KCL application shouldn't have access to anything outside of the standard KCL classes so the shardConsumerFactory and all other classes should be the Kinesis variant. Obviously in an adapter application, using the KinesisShardConsumerFactory with the DDB Streams classes will cause issues but standard KCL applications should never face that issue
Description of changes: The remaining changes require to allow for re-compatibility with the DynamoDBStreamsKinesisAdapter. The majority of changes consist of creating interfaces for key classes like the ShardConsumer, renaming these preexisting classes to include the Kinesis prefix (ShardConsumer -> KinesisShardConsumer) to distinguish from any classes that will use the new interfaces (i.e. DynamoDBStreamsShardConsumer), and making several utility classes and functions public so they could be accessed by the adapter or interfaces, such as the InitializationTask
Testing: Testing was done with the StreamsKCLAdapterCheck package which contains Kinesis KCL tests. Changes were also verified with the built in unit tests using mvn test.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.