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


##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientHandler.java:
##########
@@ -154,7 +156,15 @@ private AbfsDfsClient createDfsClient(final URL baseUrl,
       final EncryptionContextProvider encryptionContextProvider,
       final AbfsClientContext abfsClientContext) throws IOException {
     URL dfsUrl = changeUrlFromBlobToDfs(baseUrl);
-    if (tokenProvider != null) {
+    if (tokenProvider != null && sasTokenProvider != null) {

Review Comment:
   Logging as well can be combined. in log we will see null value for the one 
which is not supposed to be used and we will know.
   Better to combine everywhere IMO.



##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsBlobClient.java:
##########
@@ -201,7 +201,21 @@ public AbfsBlobClient(final URL baseUrl,
       final SASTokenProvider sasTokenProvider,
       final EncryptionContextProvider encryptionContextProvider,
       final AbfsClientContext abfsClientContext) throws IOException {
-    super(baseUrl, sharedKeyCredentials, abfsConfiguration, sasTokenProvider,
+    super(baseUrl, sharedKeyCredentials, abfsConfiguration, null, 
sasTokenProvider,
+        encryptionContextProvider, abfsClientContext, AbfsServiceType.BLOB);
+    this.azureAtomicRenameDirSet = new HashSet<>(Arrays.asList(
+        abfsConfiguration.getAzureAtomicRenameDirs()
+            .split(AbfsHttpConstants.COMMA)));
+  }
+
+  public AbfsBlobClient(final URL baseUrl,

Review Comment:
   Similarly here also we can do how we did for AbfsClient.
   We have 3 constructors here and the only diff they have is the token 
provider.
   Let's combine them and caller can choose what to send



##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -363,6 +363,21 @@ public AbfsClient(final URL baseUrl,
     this.sasTokenProvider = sasTokenProvider;
   }
 
+  public AbfsClient(final URL baseUrl,

Review Comment:
   Similar we should do for AbfsBlobClient and AbfsDfsClient



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to