[ 
https://issues.apache.org/jira/browse/HADOOP-18516?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17818638#comment-17818638
 ] 

ASF GitHub Bot commented on HADOOP-18516:
-----------------------------------------

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





> [ABFS]: Support fixed SAS token config in addition to SAS Token Provider class
> ------------------------------------------------------------------------------
>
>                 Key: HADOOP-18516
>                 URL: https://issues.apache.org/jira/browse/HADOOP-18516
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: fs/azure
>    Affects Versions: 3.3.4
>            Reporter: Sree Bhattacharyya
>            Assignee: Anuj Modi
>            Priority: Minor
>              Labels: pull-request-available
>
> Introduce a new configuration for setting the fixed account/service SAS token 
> in ABFS driver. This will be in addition to the implementations of the 
> SASTokenProvider interface that can be used for obtaining a SAS Token from 
> the user.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to