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