Github user tzulitai commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5803#discussion_r180909552
  
    --- Diff: 
flink-connectors/flink-connector-kinesis/src/main/java/org/apache/flink/streaming/connectors/kinesis/proxy/KinesisProxy.java
 ---
    @@ -176,6 +179,16 @@ protected KinesisProxy(Properties configProps) {
     
        }
     
    +   /**
    +    * Create the Kinesis client, using the provided configuration 
properties and default {@link ClientConfiguration}.
    +    * Derived classes can override this method to customize the client 
configuration.
    +    * @param configProps
    +    * @return
    +    */
    +   protected AmazonKinesis createKinesisClient(Properties configProps) {
    --- End diff --
    
    My main concern with allowing overrides of this method, is that override 
implementations can potentially completely ignore the `configProps` settings 
and create a Kinesis client entirely irrelevant from the original 
configuration. IMO, this is not nice design-wise.
    
    As a different approach, would it be possible to traverse keys in the 
`configProps` and set the `ClientConfiguration` appropriately, such that we 
won't need to be aware of all updated / new keys in the AWS Kinesis SDK? 
Ideally, Flink should not need to maintain its own set of config keys and just 
rely on AWS's keys (for example, Flink actually should not need to define its 
own config keys for AWS credentials).


---

Reply via email to