anujmodi2021 commented on code in PR #6552:
URL: https://github.com/apache/hadoop/pull/6552#discussion_r1495257805
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -941,31 +941,57 @@ public AccessTokenProvider getTokenProvider() throws
TokenAccessProviderExceptio
}
}
+ /**
+ * The following method chooses between a configured fixed sas token, and a
user implementation of the SASTokenProvider interface,
+ * depending on which one is available. In case a user SASTokenProvider
implementation is not present, and a fixed token is configured,
+ * it simply returns null, to set the sasTokenProvider object for current
configuration instance to null.
+ * The fixed token is read and used later. This is done to:
+ * 1. check for cases where both are not set, while initializing
AbfsConfiguration,
+ * to not proceed further than thi stage itself when none of the options are
available.
+ * 2. avoid using similar tokenProvider implementation to just read the
configured fixed token,
+ * as this could create confusion. The configuration is introduced
+ * primarily to avoid using any tokenProvider class/interface.
Also,implementing the SASTokenProvider requires relying on the raw
configurations.
+ * It is more stable to depend on the AbfsConfiguration with which a
filesystem is initialized,
+ * and eliminate chances of dynamic modifications and spurious situations.
+ * @return sasTokenProvider object
+ * @throws AzureBlobFileSystemException
+ */
public SASTokenProvider getSASTokenProvider() throws
AzureBlobFileSystemException {
AuthType authType = getEnum(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME,
AuthType.SharedKey);
if (authType != AuthType.SAS) {
- throw new SASTokenProviderException(String.format(
- "Invalid auth type: %s is being used, expecting SAS", authType));
+ throw new SASTokenProviderException(String.format("Invalid auth type: %s
is being used, expecting SAS", authType));
}
try {
- String configKey = FS_AZURE_SAS_TOKEN_PROVIDER_TYPE;
- Class<? extends SASTokenProvider> sasTokenProviderClass =
- getTokenProviderClass(authType, configKey, null,
+ Class<? extends SASTokenProvider> sasTokenProviderImplementation =
+ getTokenProviderClass(authType, FS_AZURE_SAS_TOKEN_PROVIDER_TYPE,
+ null,
SASTokenProvider.class);
-
- Preconditions.checkArgument(sasTokenProviderClass != null,
- String.format("The configuration value for \"%s\" is invalid.",
configKey));
-
- SASTokenProvider sasTokenProvider = ReflectionUtils
- .newInstance(sasTokenProviderClass, rawConfig);
- Preconditions.checkArgument(sasTokenProvider != null,
- String.format("Failed to initialize %s", sasTokenProviderClass));
-
- LOG.trace("Initializing {}", sasTokenProviderClass.getName());
- sasTokenProvider.initialize(rawConfig, accountName);
- LOG.trace("{} init complete", sasTokenProviderClass.getName());
- return sasTokenProvider;
+ String configuredFixedToken =
this.rawConfig.get(FS_AZURE_SAS_FIXED_TOKEN,
+ null);
+
+ Preconditions.checkArgument(!(sasTokenProviderImplementation == null
Review Comment:
Preference will be given to SASTokenProvider implementation.
Modified javadocs to make it clear
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -941,31 +941,57 @@ public AccessTokenProvider getTokenProvider() throws
TokenAccessProviderExceptio
}
}
+ /**
+ * The following method chooses between a configured fixed sas token, and a
user implementation of the SASTokenProvider interface,
+ * depending on which one is available. In case a user SASTokenProvider
implementation is not present, and a fixed token is configured,
+ * it simply returns null, to set the sasTokenProvider object for current
configuration instance to null.
+ * The fixed token is read and used later. This is done to:
+ * 1. check for cases where both are not set, while initializing
AbfsConfiguration,
+ * to not proceed further than thi stage itself when none of the options are
available.
+ * 2. avoid using similar tokenProvider implementation to just read the
configured fixed token,
+ * as this could create confusion. The configuration is introduced
+ * primarily to avoid using any tokenProvider class/interface.
Also,implementing the SASTokenProvider requires relying on the raw
configurations.
+ * It is more stable to depend on the AbfsConfiguration with which a
filesystem is initialized,
+ * and eliminate chances of dynamic modifications and spurious situations.
+ * @return sasTokenProvider object
+ * @throws AzureBlobFileSystemException
+ */
public SASTokenProvider getSASTokenProvider() throws
AzureBlobFileSystemException {
AuthType authType = getEnum(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME,
AuthType.SharedKey);
if (authType != AuthType.SAS) {
- throw new SASTokenProviderException(String.format(
- "Invalid auth type: %s is being used, expecting SAS", authType));
+ throw new SASTokenProviderException(String.format("Invalid auth type: %s
is being used, expecting SAS", authType));
}
try {
- String configKey = FS_AZURE_SAS_TOKEN_PROVIDER_TYPE;
- Class<? extends SASTokenProvider> sasTokenProviderClass =
- getTokenProviderClass(authType, configKey, null,
+ Class<? extends SASTokenProvider> sasTokenProviderImplementation =
+ getTokenProviderClass(authType, FS_AZURE_SAS_TOKEN_PROVIDER_TYPE,
+ null,
SASTokenProvider.class);
-
- Preconditions.checkArgument(sasTokenProviderClass != null,
- String.format("The configuration value for \"%s\" is invalid.",
configKey));
-
- SASTokenProvider sasTokenProvider = ReflectionUtils
- .newInstance(sasTokenProviderClass, rawConfig);
- Preconditions.checkArgument(sasTokenProvider != null,
- String.format("Failed to initialize %s", sasTokenProviderClass));
-
- LOG.trace("Initializing {}", sasTokenProviderClass.getName());
- sasTokenProvider.initialize(rawConfig, accountName);
- LOG.trace("{} init complete", sasTokenProviderClass.getName());
- return sasTokenProvider;
+ String configuredFixedToken =
this.rawConfig.get(FS_AZURE_SAS_FIXED_TOKEN,
+ null);
+
+ Preconditions.checkArgument(!(sasTokenProviderImplementation == null
+ && configuredFixedToken == null),
+ String.format(
Review Comment:
Nice!!
Taken
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
