ggershinsky commented on PR #13225: URL: https://github.com/apache/iceberg/pull/13225#issuecomment-3507798396
I'm also fine with the motivation. Regarding the support for encrypted tables, I have two concerns, 1. Unrelated to REST. What if the `encryption.key-id` table property is set (https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/TableProperties.java#L391) but a custom TO implementation ignores it. Do the users expect a table to be encrypted if the `encryption.key-id` is set? Should the implementors of custom TOs validate them by running the Iceberg unitests (inc `TestTableEncryption`)? 2. Related to REST. The standard RESTableIOperations, built in Iceberg, is verified (in this PR) to be safe wrt metadata access. A custom TO replacement can behave differently of course. I think both concerns can be addressed by adding a few lines to the `RESTOperationsBuilder` javadoc API comments (not to the REST spec, because the first comment is unrelated, and the second is related only to client impl, not the server impl) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
