bhattmanish98 commented on code in PR #7713:
URL: https://github.com/apache/hadoop/pull/7713#discussion_r2165875045


##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -58,6 +58,7 @@
 import org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore.Permissions;
 import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants;
 import org.apache.hadoop.fs.azurebfs.constants.AbfsHttpConstants.ApiVersion;
+import org.apache.hadoop.fs.azurebfs.constants.AbfsServiceType;

Review Comment:
   Instead of importing the entire AbfsServiceType class, we can use static 
import of BLOB enum.



##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java:
##########
@@ -355,6 +356,42 @@ public void verifyUserAgentClusterType() throws Exception {
       .contains(DEFAULT_VALUE_UNKNOWN);
   }
 
+  @Test
+  // Test to verify the unique identifier in user agent string for FNS-Blob 
accounts
+  public void verifyUserAgentForFNSBlob() throws Exception {
+    assumeHnsDisabled();
+    Assume.assumeTrue(getAbfsServiceType() == AbfsServiceType.BLOB);

Review Comment:
   Please use assumeBlobServiceType()



##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -1319,6 +1321,12 @@ String initializeUserAgent(final AbfsConfiguration 
abfsConfiguration,
     sb.append(FORWARD_SLASH);
     sb.append(abfsConfiguration.getClusterType());
 
+    // Add a unique identifier in FNS-Blob user agent string
+    if (!getIsNamespaceEnabled() && 
abfsConfiguration.getFsConfiguredServiceType() == AbfsServiceType.BLOB){
+      sb.append(SEMICOLON).append(SINGLE_WHITE_SPACE);
+      sb.append(FNS_BLOB_USER_AGENT_IDENTIFIER);

Review Comment:
   sb.append(SEMICOLON)
        .append(SINGLE_WHITE_SPACE)
        .append(FNS_BLOB_USER_AGENT_IDENTIFIER);



##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java:
##########
@@ -355,6 +356,42 @@ public void verifyUserAgentClusterType() throws Exception {
       .contains(DEFAULT_VALUE_UNKNOWN);
   }
 
+  @Test
+  // Test to verify the unique identifier in user agent string for FNS-Blob 
accounts
+  public void verifyUserAgentForFNSBlob() throws Exception {
+    assumeHnsDisabled();
+    Assume.assumeTrue(getAbfsServiceType() == AbfsServiceType.BLOB);
+    final AzureBlobFileSystem fs = getFileSystem(getRawConfiguration());
+    final AbfsConfiguration configuration = fs.getAbfsStore()
+        .getAbfsConfiguration();
+
+    String userAgentStr = getUserAgentString(configuration, false);
+    verifyBasicInfo(userAgentStr);
+    Assertions.assertThat(userAgentStr)
+        .describedAs(
+            "User-Agent string for FNS accounts on Blob endpoint should 
contain "
+                + FNS_BLOB_USER_AGENT_IDENTIFIER)
+        .contains(FNS_BLOB_USER_AGENT_IDENTIFIER);
+  }
+
+  @Test
+  // Test to verify that the user agent string for non-FNS-Blob accounts
+  // does not contain the FNS identifier.
+  public void verifyUserAgentForDFS() throws Exception {
+    Assume.assumeTrue(getAbfsServiceType() == AbfsServiceType.DFS);
+    final AzureBlobFileSystem fs = getFileSystem(getRawConfiguration());

Review Comment:
   same as above.



##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/ITestAbfsClient.java:
##########
@@ -355,6 +356,42 @@ public void verifyUserAgentClusterType() throws Exception {
       .contains(DEFAULT_VALUE_UNKNOWN);
   }
 
+  @Test
+  // Test to verify the unique identifier in user agent string for FNS-Blob 
accounts
+  public void verifyUserAgentForFNSBlob() throws Exception {
+    assumeHnsDisabled();
+    Assume.assumeTrue(getAbfsServiceType() == AbfsServiceType.BLOB);
+    final AzureBlobFileSystem fs = getFileSystem(getRawConfiguration());

Review Comment:
   Just getFileSystem() is also enough incase there is no other config you want 
to add in the raw configuration.



##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -1319,6 +1321,12 @@ String initializeUserAgent(final AbfsConfiguration 
abfsConfiguration,
     sb.append(FORWARD_SLASH);
     sb.append(abfsConfiguration.getClusterType());
 
+    // Add a unique identifier in FNS-Blob user agent string
+    if (!getIsNamespaceEnabled() && 
abfsConfiguration.getFsConfiguredServiceType() == AbfsServiceType.BLOB){

Review Comment:
   With static import, we just need to give BLOB, instead of 
AbfsServiceType.BLOB



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