-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19284: [ABFS] Allow "fs.azure.account.hns.enabled" to be set as Account Specific Config #7062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HADOOP-19284: [ABFS] Allow "fs.azure.account.hns.enabled" to be set as Account Specific Config #7062
Conversation
🎊 +1 overall
This message was automatically generated. |
Assertions.assertThat(getIsNamespaceEnabled(fs2)).describedAs( | ||
"getIsNamespaceEnabled should return true when the " | ||
+ "account specific config is set as true").isFalse(); | ||
Assertions.assertThat(getIsNamespaceEnabled(fs3)).describedAs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assertion statement should be changed right ? If the config is not set it does getAcl to determine the account type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this comment was before my latest commit. So, it might be not applicable now.
But still, we are setting account-agnostic setting here so getAcl won't be needed.
🎊 +1 overall
This message was automatically generated. |
...tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestGetNameSpaceEnabled.java
Show resolved
Hide resolved
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java
Outdated
Show resolved
Hide resolved
...ls/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemE2E.java
Show resolved
Hide resolved
...tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestGetNameSpaceEnabled.java
Outdated
Show resolved
Hide resolved
...tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestGetNameSpaceEnabled.java
Outdated
Show resolved
Hide resolved
🎊 +1 overall
This message was automatically generated. |
return Trilean.getTrilean(isNamespaceEnabledAccount); | ||
String isNamespaceEnabledAccountString | ||
= getString(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, isNamespaceEnabledAccount); | ||
return Trilean.getTrilean(isNamespaceEnabledAccountString); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the variable isNamespaceEnabledAccountString is used at one place only, shouldn't it be better to make it inplace call instead of creating new variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that sounds better
Thank you for the reviews @surendralilhore @bhattmanish98 @anmolanmol1234 I have also refactored the test added to reduce code redundancy and improve scenario coverage. Hope this looks better now. |
Updated test results: ============================================================ |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM
…s Account Specific Config (apache#7062)
Description of PR
Jira: https://issues.apache.org/jira/browse/HADOOP-19284
Making config
fs.azure.account.hns.enabled
account specificThere are a few reported requirements where users working with multiple file systems need to specify this config either only for some accounts or set it differently for different account.
ABFS driver today does not allow this to be set as account specific config.
This also fixes test that were failing when "fs.azure.account.hns.enabled" config was not present.
How was this patch tested?
Existing tests modified and new tests added.
Test results:
Failing tests are known and we are working on the fixes.
HNS-OAuth
[ERROR] testFileSystemCapabilities(org.apache.hadoop.fs.azurebfs.ITestFileSystemInitialization) Time elapsed: 3.079 s <<< FAILURE!
[ERROR] testEtagConsistencyAcrossListAndHead(org.apache.hadoop.fs.azurebfs.contract.ITestAbfsFileSystemContractEtag) Time elapsed: 0.307 s <<< FAILURE!
[ERROR] testDeleteTargetPaths(org.apache.hadoop.fs.azurebfs.commit.ITestAbfsRenameStageFailure) Time elapsed: 4.837 s <<< FAILURE!
[ERROR] testReadAndWriteWithDifferentBufferSizesAndSeekSize=104,857,600-readahead=true-Client=JDK_HTTP_URL_CONNECTION Time elapsed: 204.267 s <<< FAILURE!
[ERROR] testBackoffRetryMetrics(org.apache.hadoop.fs.azurebfs.services.TestAbfsRestOperation) Time elapsed: 3.428 s <<< ERROR!
[ERROR] testReadFooterMetrics(org.apache.hadoop.fs.azurebfs.ITestAbfsReadFooterMetrics) Time elapsed: 0.527 s <<< ERROR!
[ERROR] testMetricWithIdlePeriod(org.apache.hadoop.fs.azurebfs.ITestAbfsReadFooterMetrics) Time elapsed: 0.55 s <<< ERROR!
[ERROR] testReadFooterMetricsWithParquetAndNonParquet(org.apache.hadoop.fs.azurebfs.ITestAbfsReadFooterMetrics) Time elapsed: 0.583 s <<< ERROR!
[ERROR] testComplexDirActions(org.apache.hadoop.fs.azurebfs.contract.ITestAbfsFileSystemContractGetFileStatus) Time elapsed: 180.02 s <<< ERROR!
[ERROR] test_120_terasort(org.apache.hadoop.fs.azurebfs.commit.ITestAbfsTerasort) Time elapsed: 1.947 s <<< ERROR!