snvijaya commented on a change in pull request #2123:
URL: https://github.com/apache/hadoop/pull/2123#discussion_r453584579



##########
File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -77,11 +77,14 @@ private AzureADAuthenticator() {
    * @param clientId     the client ID (GUID) of the client web app
    *                     btained from Azure Active Directory configuration
    * @param clientSecret the secret key of the client web app
+   * @param tokenFetchRetryPolicy retry policy to be used for token fetch AAD
+   *                             calls.
    * @return {@link AzureADToken} obtained using the creds
    * @throws IOException throws IOException if there is a failure in 
connecting to Azure AD
    */
   public static AzureADToken getTokenUsingClientCreds(String authEndpoint,
-                                                      String clientId, String 
clientSecret)
+                                                      String clientId,
+      String clientSecret, ExponentialRetryPolicy tokenFetchRetryPolicy)

Review comment:
       Indentation.

##########
File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -275,21 +287,24 @@ public UnexpectedResponseException(final int 
httpErrorCode,
   }
 
   private static AzureADToken getTokenCall(String authEndpoint, String body,
-      Hashtable<String, String> headers, String httpMethod) throws IOException 
{
-    return getTokenCall(authEndpoint, body, headers, httpMethod, false);
+      Hashtable<String, String> headers, String httpMethod,
+      ExponentialRetryPolicy tokenFetchRetryPolicy) throws IOException {
+    return getTokenCall(authEndpoint, body, headers, httpMethod, false,
+        tokenFetchRetryPolicy);
   }
 
   private static AzureADToken getTokenCall(String authEndpoint, String body,
-      Hashtable<String, String> headers, String httpMethod, boolean isMsi)
+      Hashtable<String, String> headers, String httpMethod, boolean isMsi,
+      ExponentialRetryPolicy tokenFetchRetryPolicy)
       throws IOException {
     AzureADToken token = null;
-    ExponentialRetryPolicy retryPolicy
-            = new ExponentialRetryPolicy(3, 0, 1000, 2);
 
     int httperror = 0;
     IOException ex = null;
     boolean succeeded = false;
     int retryCount = 0;
+    boolean shouldRetry;
+    LOG.debug("First execution of REST operation getTokenSingleCall");

Review comment:
       Make this a trace level log.

##########
File path: 
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/oauth2/AzureADAuthenticator.java
##########
@@ -275,21 +287,24 @@ public UnexpectedResponseException(final int 
httpErrorCode,
   }
 
   private static AzureADToken getTokenCall(String authEndpoint, String body,
-      Hashtable<String, String> headers, String httpMethod) throws IOException 
{
-    return getTokenCall(authEndpoint, body, headers, httpMethod, false);
+      Hashtable<String, String> headers, String httpMethod,
+      ExponentialRetryPolicy tokenFetchRetryPolicy) throws IOException {
+    return getTokenCall(authEndpoint, body, headers, httpMethod, false,
+        tokenFetchRetryPolicy);
   }
 
   private static AzureADToken getTokenCall(String authEndpoint, String body,
-      Hashtable<String, String> headers, String httpMethod, boolean isMsi)
+      Hashtable<String, String> headers, String httpMethod, boolean isMsi,
+      ExponentialRetryPolicy tokenFetchRetryPolicy)
       throws IOException {
     AzureADToken token = null;
-    ExponentialRetryPolicy retryPolicy

Review comment:
       As every OAuth token provider eventually calls this method to get token, 
you will just need to create an instance of exponential retry right here with 
the configured values ? 
   Is there any benefit creating and passing down exponential retry instance 
from each of the token provider ?




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