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