steveloughran commented on a change in pull request #3620:
URL: https://github.com/apache/hadoop/pull/3620#discussion_r743586220
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java
##########
@@ -1411,6 +1405,30 @@ AzureBlobFileSystemStore getAbfsStore() {
return abfsStore;
}
+ @VisibleForTesting
+ void setAbfsStore(AzureBlobFileSystemStore abfsStore) {
+ this.abfsStore = abfsStore;
+ }
+
+ @VisibleForTesting
+ void createFileSystemIfNotExist(TracingContext tracingContext) throws
IOException {
+ if (this.tryGetFileStatus(new Path(AbfsHttpConstants.ROOT_PATH),
tracingContext) == null) {
+ try {
+ this.createFileSystem(tracingContext);
+ } catch (IOException ex) {
+ if (ex instanceof AzureBlobFileSystemException) {
+ checkException(null, (AzureBlobFileSystemException) ex,
+ AzureServiceErrorCode.FILE_SYSTEM_ALREADY_EXISTS);
+ } else if (ex.getCause() != null && ex.getCause() instanceof
AzureBlobFileSystemException) {
Review comment:
instanceof always returns false if source is null, so the null check
here can be removed
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java
##########
@@ -49,4 +56,39 @@ public void
ensureFilesystemWillNotBeCreatedIfCreationConfigIsNotSet() throws Ex
final AzureBlobFileSystem fs = this.createFileSystem();
FileStatus[] fileStatuses = fs.listStatus(new Path("/"));
}
+
+ @Test
+ public void ensureFilesystemWillBeCreatedIfCreationConfigIsSet() throws
Exception {
+ final AzureBlobFileSystem fs = createFileSystem();
Review comment:
use try with resources to close afterwards
##########
File path:
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/ITestAzureBlobFileSystemInitAndCreate.java
##########
@@ -49,4 +56,39 @@ public void
ensureFilesystemWillNotBeCreatedIfCreationConfigIsNotSet() throws Ex
final AzureBlobFileSystem fs = this.createFileSystem();
FileStatus[] fileStatuses = fs.listStatus(new Path("/"));
}
+
+ @Test
+ public void ensureFilesystemWillBeCreatedIfCreationConfigIsSet() throws
Exception {
+ final AzureBlobFileSystem fs = createFileSystem();
+ Configuration config = getRawConfiguration();
+ fs.initialize(FileSystem.getDefaultUri(config), config);
+
+ // Make sure createFileSystemIfNotExists is working as intended.
+ final MockAzureBlobFileSystemStore store = new
MockAzureBlobFileSystemStore(config);
+ fs.setAbfsStore(store);
+ fs.createFileSystemIfNotExist(getTestTracingContext(fs, true));
+ assert(store.isCreateFileSystemCalled);
Review comment:
assertj assertions with useful error text
##########
File path:
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AzureBlobFileSystem.java
##########
@@ -1411,6 +1405,30 @@ AzureBlobFileSystemStore getAbfsStore() {
return abfsStore;
}
+ @VisibleForTesting
+ void setAbfsStore(AzureBlobFileSystemStore abfsStore) {
+ this.abfsStore = abfsStore;
+ }
+
+ @VisibleForTesting
+ void createFileSystemIfNotExist(TracingContext tracingContext) throws
IOException {
+ if (this.tryGetFileStatus(new Path(AbfsHttpConstants.ROOT_PATH),
tracingContext) == null) {
+ try {
+ this.createFileSystem(tracingContext);
+ } catch (IOException ex) {
+ if (ex instanceof AzureBlobFileSystemException) {
Review comment:
better to add a catch(AzureBlobFileSystemException) clause
--
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]