gszadovszky commented on a change in pull request #842:
URL: https://github.com/apache/parquet-mr/pull/842#discussion_r519957562
##########
File path:
parquet-hadoop/src/main/java/org/apache/parquet/crypto/keytools/KeyToolkit.java
##########
@@ -206,9 +206,10 @@ String getEncodedWrappedKEK() {
public static void rotateMasterKeys(String folderPath, Configuration
hadoopConfig)
throws IOException, ParquetCryptoRuntimeException,
KeyAccessDeniedException, UnsupportedOperationException {
- if (hadoopConfig.getBoolean(KEY_MATERIAL_INTERNAL_PROPERTY_NAME,
KEY_MATERIAL_INTERNAL_DEFAULT)) {
+ if (hadoopConfig.getBoolean(KEY_MATERIAL_INTERNAL_PROPERTY_NAME, false)) {
throw new UnsupportedOperationException("Key rotation is not supported
for internal key material");
}
+ hadoopConfig.setBoolean(KEY_MATERIAL_INTERNAL_PROPERTY_NAME, false);
Review comment:
It is not common to set a value in the hadoop conf in our code base. I
would expect some comments about why this is necessary. Also, I am not sure if
this would not impact the caller. If the caller gives us a hadoop Configuration
object we should not modify it.
----------------------------------------------------------------
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]
