Added IKinesisProxy injector in Worker.Builder #274
Conversation
…tom proxy implementations
| * Set KinesisProxy for the worker. | ||
| * | ||
| * @param kinesisProxy | ||
| * KinesisProxy uses the AmazonKinesis client to get data from Kinesis or DynamoDBStreams |
There was a problem hiding this comment.
Can you fix this comment, it would be confusing for the customers since KCL already has DDB endpoints. Can you remove DynaoDBStreams from the comment.
This reverts commit aa944c1. Reverting to undo changes to import ordering.
Re-added unit tests after reverting changes to import ordering.
| @@ -1252,18 +1269,20 @@ public Worker build() { | |||
| } | |||
| } | |||
| if (metricsFactory == null) { | |||
| metricsFactory = getMetricsFactory(cloudWatchClient, config); | |||
| metricsFactory = Worker.getMetricsFactory(cloudWatchClient, config); | |||
There was a problem hiding this comment.
Why is this being changed? The getMetricsFactory method is a static method which is accessible within the same class.
There was a problem hiding this comment.
It was changed because the Lombok Getter had the same name as the static method.
| .config(config); | ||
| builder.build(); | ||
| assertNotNull(builder.getKinesisProxy()); | ||
| assertTrue(builder.getKinesisProxy() instanceof KinesisProxy); |
There was a problem hiding this comment.
This is not the correct way to test the proxy that is set. We need to make sure that the worker is getting the proxy that we expect it to use.
| } | ||
|
|
||
| // KinesisProxyImplementation to test KinesisProxy injection. | ||
| private class DummyKinesisProxy implements IKinesisProxy { |
There was a problem hiding this comment.
We should be able to use KinesisLocalFileProxy instead of creating a new one. Let me get back to you on that one.
| @@ -1062,6 +1064,7 @@ private static ExecutorService getExecutorService() { | |||
| /** | |||
| * Builder to construct a Worker instance. | |||
| */ | |||
| @Getter | |||
There was a problem hiding this comment.
There is no use for the builder to have a getter, we should be testing what the builder has created, if that has the correct value.
* Allow providing a custom IKinesisProxy implementation. * PR awslabs#274 * Checkpointing on a different thread should no longer emit a warning about NullMetricsScope. * PR awslabs#284 * Issue awslabs#48 * Upgraded the AWS Java SDK to version 1.11.271 * PR awslabs#287
Added IKinesisProxy injector in Worker.Builder to allow injecting custom proxy implementations