[GitHub] [iceberg] danielcweeks commented on pull request #4714: [AWS]Add AwsKmsClient for table encryption KMS client implementation

[GitBox <gi...@apache.org>]

danielcweeks commented on PR #4714:
URL: https://github.com/apache/iceberg/pull/4714#issuecomment-1121304920

   Hey @JiJiTang I'm hoping you can clarify a little more about this implementation and the use case.  A few things confuse me a little:
   
   1. This appears to be a standalone kms client implementation, but I don't see how anyone could use this directly without bringing custom code (i.e. we don't dynamically load any of the aws clients individually).
   2. This doesn't appear to be implemented in the same pattern as the AWSClientFactory (I see there is a factory method on this implementation, but that's quite different from how other clients are accessed).
   
   There are a lot of different configurations with AWS clients and I don't think the intent is to make Iceberg support every option, so it might be better to just maintain your own client factory with customizations like this.
   
   If we were to support key generation, it would probably need to be integrated with the AWSClientFactory implementations and configurable via a property, but this seems to be getting into pretty narrow use cases that add additional complexity.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org