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