jackye1995 commented on pull request #1531: URL: https://github.com/apache/iceberg/pull/1531#issuecomment-701743469
I have 2 concerns with this PR: 1. I think we should in addition propose dynamically loading the `LocationProvider` implementation, instead of using a boolean to switch on or off each implementation. This will allow any user to easily plugin a custom implementation. I wonder if such proposal is enough to both satisfy your need and also keep the community version general. 2. for this specific implementation, the approach of using table property to get `localizedDataLocationLookUp` map feels not scalable to me if we want to make it a useful generic implementation. For example, customers might want to have 1 HDFS cluster per [AWS local zone](https://aws.amazon.com/about-aws/global-infrastructure/localzones/) to read and write data due to latency requirement, and this setup easily generate hundreds to thousands of table properties. I don't know if there is any bad impact for such use case, since table properties is not designed to hold a lot of data. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
