anujmodi2021 commented on code in PR #8152:
URL: https://github.com/apache/hadoop/pull/8152#discussion_r2664384142
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java:
##########
@@ -315,6 +316,14 @@ public void initialize(URI uri, Configuration
configuration)
throw new
InvalidConfigurationValueException(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, ex);
}
+ // For FNS-DFS accounts, reset the endpoint to Blob and update the tracing
Review Comment:
Nit: use block comment as above
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java:
##########
@@ -160,4 +164,53 @@ public void
testFileSystemInitFailsIfNotAbleToDetermineAccountType() throws Exce
FS_AZURE_ACCOUNT_IS_HNS_ENABLED, () ->
mockedFs.initialize(fs.getUri(), getRawConfiguration()));
}
+
+ /**
+ * Test to verify that the fnsEndptConvertedIndicator ("T") is present in
the tracing header
+ * after endpoint conversion during AzureBlobFileSystem initialization.
+ *
+ * @throws Exception if any error occurs during the test
+ */
+ @Test
+ public void testFNSEndptConvertedIndicatorInHeader() throws Exception {
+ assumeHnsDisabled();
+ String scheme = "abfs";
+ String dfsDomain = "dfs.core.windows.net";
+ String endptConversionIndicatorInTc = "T";
+ Configuration conf = new Configuration(getRawConfiguration());
+ conf.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION,
true);
+
+ String dfsUri = String.format("%s://%s@%s.%s/",
+ scheme, getFileSystemName(),
+ getAccountName().substring(0, getAccountName().indexOf(DOT)),
+ dfsDomain);
+
+ // Initialize filesystem with DFS endpoint
+ AzureBlobFileSystem fs =
Review Comment:
Should be inside try to autoclose
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -752,6 +752,15 @@ public AbfsServiceType getFsConfiguredServiceType() {
return getCaseInsensitiveEnum(FS_AZURE_FNS_ACCOUNT_SERVICE_TYPE,
fsConfiguredServiceType);
}
+ /**
+ * Returns the service type identified from the URL used to initialize the
FileSystem.
+ *
+ * @return the configured AbfsServiceType from the URL
+ */
+ public AbfsServiceType getFsConfiguredServiceTypeFromURL() {
+ return fsConfiguredServiceType;
Review Comment:
Nit: Better to change this variable name also to
`fsConfiguredServiceTypeFromUrl`
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -1799,6 +1808,14 @@ void setReadAheadEnabled(final boolean enabledReadAhead)
{
this.enabledReadAhead = enabledReadAhead;
}
+ /**
+ * Sets the configured service type to BLOB.
+ * Majorly required to correctly set user agent for FNS-Blob
+ */
+ void setFsConfiguredServiceTypetoBlob() {
Review Comment:
Nit: camel case in method name
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystemStore.java:
##########
@@ -1906,6 +1909,17 @@ private AbfsPerfInfo startTracking(String callerName,
String calleeName) {
return new AbfsPerfInfo(abfsPerfTracker, callerName, calleeName);
}
+ /**
+ * Resets all service types to use BLOB.
+ * Updates the client to reflect the new default service type.
+ */
+ public void resetEndpointforFNS() {
Review Comment:
Nit: camel case in method name
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -1799,6 +1808,14 @@ void setReadAheadEnabled(final boolean enabledReadAhead)
{
this.enabledReadAhead = enabledReadAhead;
}
+ /**
+ * Sets the configured service type to BLOB.
+ * Majorly required to correctly set user agent for FNS-Blob
+ */
+ void setFsConfiguredServiceTypetoBlob() {
Review Comment:
Better to have generic method that can accept Service Type and caller can
call it with Blob
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java:
##########
@@ -156,6 +156,7 @@ public class AzureBlobFileSystem extends FileSystem
*/
private boolean isClosed = true;
private final String fileSystemId = UUID.randomUUID().toString();
+ private final String DFS_DOMAIN_INDICATOR = ".dfs.";
Review Comment:
+1
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientHandler.java:
##########
@@ -86,11 +86,47 @@ public AbfsClientHandler(final URL baseUrl,
* Initialize the default service type based on the user configuration.
* @param abfsConfiguration set by user.
*/
- private void initServiceType(final AbfsConfiguration abfsConfiguration) {
+ public void initServiceType(final AbfsConfiguration abfsConfiguration) {
this.defaultServiceType = abfsConfiguration.getFsConfiguredServiceType();
this.ingressServiceType = abfsConfiguration.getIngressServiceType();
}
+ /**
+ * Sets the default service type.
+ *
+ * @param defaultServiceType the service type to set as default
+ */
+ public void setDefaultServiceType(AbfsServiceType defaultServiceType) {
+ this.defaultServiceType = defaultServiceType;
+ }
+
+ /**
+ * Sets the ingress service type.
+ *
+ * @param ingressServiceType the ingress service type
+ */
+ public void setIngressServiceType(AbfsServiceType ingressServiceType) {
+ this.ingressServiceType = ingressServiceType;
+ }
+
+ /**
+ * Gets the default service type.
+ *
+ * @return the default service type
+ */
+ public AbfsServiceType getDefaultServiceType() {
+ return defaultServiceType;
+ }
+
+ /**
+ * Gets the default ingress service type.
+ *
+ * @return the default ingress service type
+ */
+ public AbfsServiceType getDefaultIngressServiceType() {
Review Comment:
methond name should be `getngressServiceType()`
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClientHandler.java:
##########
@@ -86,11 +86,47 @@ public AbfsClientHandler(final URL baseUrl,
* Initialize the default service type based on the user configuration.
* @param abfsConfiguration set by user.
*/
- private void initServiceType(final AbfsConfiguration abfsConfiguration) {
+ public void initServiceType(final AbfsConfiguration abfsConfiguration) {
this.defaultServiceType = abfsConfiguration.getFsConfiguredServiceType();
this.ingressServiceType = abfsConfiguration.getIngressServiceType();
}
+ /**
+ * Sets the default service type.
+ *
+ * @param defaultServiceType the service type to set as default
+ */
+ public void setDefaultServiceType(AbfsServiceType defaultServiceType) {
+ this.defaultServiceType = defaultServiceType;
+ }
+
+ /**
+ * Sets the ingress service type.
+ *
+ * @param ingressServiceType the ingress service type
+ */
+ public void setIngressServiceType(AbfsServiceType ingressServiceType) {
+ this.ingressServiceType = ingressServiceType;
+ }
+
+ /**
+ * Gets the default service type.
+ *
+ * @return the default service type
+ */
+ public AbfsServiceType getDefaultServiceType() {
Review Comment:
Not used, can be removed
--
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]
