shameersss1 commented on code in PR #6884:
URL: https://github.com/apache/hadoop/pull/6884#discussion_r1832693922
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/EncryptionS3ClientFactory.java:
##########
@@ -246,8 +255,13 @@ private S3AsyncClient
createS3AsyncEncryptionClient(S3ClientCreationParameters p
s3EncryptionAsyncClientBuilder.cryptoMaterialsManager(kmsCryptoMaterialsManager);
break;
case CUSTOM:
- Keyring keyring =
getKeyringProvider(cseMaterials.getCustomKeyringClassName(),
- cseMaterials.getConf());
+ Keyring keyring;
+ try {
+ keyring =
+ getKeyringProvider(cseMaterials.getCustomKeyringClassName(),
cseMaterials.getConf());
+ } catch (RuntimeException e) {
+ throw new IOException("Failed to instantiate a custom keyring
provider", e);
Review Comment:
Make sense!
ack.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/EncryptionS3ClientFactory.java:
##########
@@ -260,21 +274,32 @@ private S3AsyncClient
createS3AsyncEncryptionClient(S3ClientCreationParameters p
return s3EncryptionAsyncClientBuilder.build();
}
-
/**
- * Retrieves an instance of the Keyring provider based on the provided class
name.
+ * Creates and returns a Keyring provider instance based on the given class
name.
+ *
+ * <p>This method attempts to instantiate a Keyring provider using
reflection. It first tries
+ * to create an instance using the standard ReflectionUtils.newInstance
method. If that fails,
+ * it falls back to an alternative instantiation method, which is primarily
used for testing
+ * purposes (specifically for CustomKeyring.java).
*
- * @param className The fully qualified class name of the Keyring provider
implementation.
- * @param conf The Configuration object containing the necessary
configuration properties.
- * @return An instance of the Keyring provider.
+ * @param className The fully qualified class name of the Keyring provider
to instantiate.
+ * @param conf The Configuration object to be passed to the Keyring provider
constructor.
+ * @return An instance of the specified Keyring provider.
+ * @throws RuntimeException If unable to create the Keyring provider
instance.
*/
- private Keyring getKeyringProvider(String className, Configuration conf) {
+ private Keyring getKeyringProvider(String className, Configuration conf) {
+ Class<? extends Keyring> keyringProviderClass =
getCustomKeyringProviderClass(className);
try {
- return
ReflectionUtils.newInstance(getCustomKeyringProviderClass(className), conf);
+ return ReflectionUtils.newInstance(keyringProviderClass, conf);
} catch (Exception e) {
- // this is for testing purpose to support CustomKeyring.java
- return
ReflectionUtils.newInstance(getCustomKeyringProviderClass(className), conf,
- new Class[] {Configuration.class}, conf);
+ LOG.warn("Failed to create Keyring provider", e);
+ // This is for testing purposes to support CustomKeyring.java
+ try {
+ return ReflectionUtils.newInstance(keyringProviderClass, conf,
+ new Class[] {Configuration.class}, conf);
+ } catch (Exception ex) {
+ throw new RuntimeException("Failed to create Keyring provider", ex);
Review Comment:
ack
--
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]
