steveloughran commented on code in PR #4758:
URL: https://github.com/apache/hadoop/pull/4758#discussion_r954766717
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/commit/ITestAbfsRenameStageFailure.java:
##########
@@ -59,7 +66,10 @@ protected AbstractFSContract createContract(final
Configuration conf) {
@Override
protected boolean requireRenameResilience() {
- return true;
+ if (isNamespaceEnabled()) {
Review Comment:
can be simplified to
```
return isNamespaceEnabled();
```
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/commit/ITestAbfsRenameStageFailure.java:
##########
@@ -41,6 +42,12 @@ public ITestAbfsRenameStageFailure() throws Exception {
binding = new ABFSContractTestBinding();
}
+ protected boolean isNamespaceEnabled() {
+ FileSystem fs = getFileSystem();
+ String namespaceEnabled =
fs.getConf().get("fs.azure.test.namespace.enabled");
Review Comment:
there's a Configuration.getBoolean(key, defval) which should be used for
evaluating true/false. it knows about trimming strings, case independence etc.
have a look at the class and see what else it can do. using the basic get()
without a default is generally a rare action.
better.
```
return conf.getBoolean("fs.azure.test.namespace.enabled", false)
```
2. other tests, specificlly everything under `AbstractAbfsIntegrationTest`,
probe the fs for having ACLs and use that to dynamically determine fs
capabilities. see `getIsNamespaceEnabled()` for how it is done.
i would prefer you copy the AbstractAbfsIntegrationTest algorithm as it is
based on the actual FS capabilities
--
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]
