anujmodi2021 commented on code in PR #6552:
URL: https://github.com/apache/hadoop/pull/6552#discussion_r1497042989


##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -941,33 +941,66 @@ public AccessTokenProvider getTokenProvider() throws 
TokenAccessProviderExceptio
     }
   }
 
+  /**
+   * The user can choose between a configured fixed sas token, and a user
+   * implementation of the SASTokenProvider interface. Preference will be given
+   * to SASTokenProvider class provided as the value of 
"fs.azure.sas.token.provider.type".
+   * If above config is not set, it is expected that user wants to use a
+   * fixed SAS Token provided as value of "fs.azure.sas.fixed.token".
+   * <ol>
+   *   <li>If both the configs are not provided,
+   *   initialization fails and {@link TokenAccessProviderException} is 
thrown.</li>
+   *   <li>If both are present, SASTokenProvider class will be used to 
generate SAS Token.</li>
+   *   <li>If only fixed SAS Token is configured, this will return null
+   *   and Fixed SAS token will be used to sign requests.</li>
+   * </ol>
+   * Avoid using a tokenProvider implementation just to read the configured 
fixed token,
+   * as this could create confusion. 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));
+          "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(

Review Comment:
   Setting configurations is upto users and it will bee difficult to stop him 
from setting both.
   We have clear documentation in abfs,md file as well as javadocs that 
SASTokenProvider will have preference.



-- 
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

Reply via email to