Tom-Newton commented on code in PR #38708:
URL: https://github.com/apache/arrow/pull/38708#discussion_r1396338372
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -611,6 +621,109 @@ class AzureFileSystem::Impl {
RETURN_NOT_OK(ptr->Init());
return ptr;
}
+
+ Status CreateDir(const AzurePath& path) {
+ if (path.container.empty()) {
+ return Status::Invalid("Cannot create an empty container");
+ }
+
+ if (path.path_to_file.empty()) {
+ auto container_client =
+ blob_service_client_->GetBlobContainerClient(path.container);
+ try {
+ auto response = container_client.Create();
+ if (response.Value.Created) {
+ return Status::OK();
+ } else {
+ return StatusFromErrorResponse(
+ container_client.GetUrl(), response.RawResponse.get(),
+ "Failed to create a container: " + path.container);
+ }
+ } catch (const Azure::Storage::StorageException& exception) {
+ return internal::ExceptionToStatus(
+ "Failed to create a container: " + path.container + ": " +
+ container_client.GetUrl(),
+ exception);
+ }
+ }
+
+ ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled,
+ hierarchical_namespace_.Enabled(path.container));
+ if (!hierarchical_namespace_enabled) {
+ // We can't create a directory without hierarchical namespace
+ // support. There is only "virtual directory" without
+ // hierarchical namespace support. And a "virtual directory" is
+ // (virtually) created a blob with ".../.../blob" blob name
+ // automatically.
+ return Status::OK();
+ }
+
+ auto directory_client =
datalake_service_client_->GetFileSystemClient(path.container)
+ .GetDirectoryClient(path.path_to_file);
+ try {
+ auto response = directory_client.Create();
+ if (response.Value.Created) {
+ return Status::OK();
+ } else {
+ return StatusFromErrorResponse(
+ directory_client.GetUrl(), response.RawResponse.get(),
+ "Failed to create a directory: " + path.path_to_file);
+ }
+ } catch (const Azure::Storage::StorageException& exception) {
+ return internal::ExceptionToStatus(
+ "Failed to create a directory: " + path.path_to_file + ": " +
+ directory_client.GetUrl(),
+ exception);
+ }
+ }
+
+ Status CreateDirRecursive(const AzurePath& path) {
+ if (path.container.empty()) {
+ return Status::Invalid("Cannot create an empty container");
+ }
+
+ auto container_client =
blob_service_client_->GetBlobContainerClient(path.container);
+ try {
+ container_client.CreateIfNotExists();
+ } catch (const Azure::Storage::StorageException& exception) {
+ return internal::ExceptionToStatus(
+ "Failed to create a container: " + path.container + " (" +
+ container_client.GetUrl() + ")",
+ exception);
+ }
+
+ ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled,
+ hierarchical_namespace_.Enabled(path.container));
+ if (!hierarchical_namespace_enabled) {
+ // We can't create a directory without hierarchical namespace
+ // support. There is only "virtual directory" without
+ // hierarchical namespace support. And a "virtual directory" is
+ // (virtually) created a blob with ".../.../blob" blob name
+ // automatically.
+ return Status::OK();
+ }
+
+ std::string current_path;
+ for (const auto& part : path.path_to_file_parts) {
Review Comment:
I just did a quick test. It seems like
`directory_client.CreateIfNotExists()` actually creates the directories
recursively be default, so probably we don't need this for loop.
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -611,6 +621,109 @@ class AzureFileSystem::Impl {
RETURN_NOT_OK(ptr->Init());
return ptr;
}
+
+ Status CreateDir(const AzurePath& path) {
+ if (path.container.empty()) {
+ return Status::Invalid("Cannot create an empty container");
+ }
+
+ if (path.path_to_file.empty()) {
+ auto container_client =
+ blob_service_client_->GetBlobContainerClient(path.container);
+ try {
+ auto response = container_client.Create();
+ if (response.Value.Created) {
+ return Status::OK();
+ } else {
+ return StatusFromErrorResponse(
+ container_client.GetUrl(), response.RawResponse.get(),
+ "Failed to create a container: " + path.container);
+ }
+ } catch (const Azure::Storage::StorageException& exception) {
+ return internal::ExceptionToStatus(
+ "Failed to create a container: " + path.container + ": " +
+ container_client.GetUrl(),
+ exception);
+ }
+ }
+
+ ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled,
+ hierarchical_namespace_.Enabled(path.container));
+ if (!hierarchical_namespace_enabled) {
+ // We can't create a directory without hierarchical namespace
+ // support. There is only "virtual directory" without
+ // hierarchical namespace support. And a "virtual directory" is
+ // (virtually) created a blob with ".../.../blob" blob name
+ // automatically.
Review Comment:
Personally I would reword this a bit. I wouldn't say that anything is
created automatically. Blob storage clients just tend to imply directories when
they see `/`s in the blob names.
```suggestion
// Without hierarchical namespace enabled Azure blob storage has no
directories.
// Therefore we can't, and don't need to create one. Simply creating a
blob with `/` in the name
// implies directories.
```
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -611,6 +611,110 @@ class AzureFileSystem::Impl {
RETURN_NOT_OK(ptr->Init());
return ptr;
}
+
+ Status CreateDir(const AzurePath& path) {
+ if (path.container.empty()) {
+ return Status::Invalid("Cannot create an empty container");
+ }
+
+ if (path.path_to_file.empty()) {
+ auto container_client =
+ blob_service_client_->GetBlobContainerClient(path.container);
+ try {
+ auto response = container_client.Create();
+ if (response.Value.Created) {
+ return Status::OK();
+ } else {
+ const auto& body = response.RawResponse->GetBody();
+ std::string_view body_text(reinterpret_cast<const
char*>(body.data()),
+ body.size());
+ return Status::IOError("Failed to create a container: ",
path.container, " (",
+ container_client.GetUrl(),
+ "): ",
response.RawResponse->GetReasonPhrase(), " (",
+
static_cast<int>(response.RawResponse->GetStatusCode()),
+ "): ", body_text);
+ }
+ } catch (const Azure::Storage::StorageException& exception) {
+ return internal::ExceptionToStatus(
+ "Failed to create a container: " + path.container + " (" +
+ container_client.GetUrl() + ")",
+ exception);
+ }
+ }
+
+ ARROW_ASSIGN_OR_RAISE(auto hierarchical_namespace_enabled,
+ hierarchical_namespace_.Enabled(path.container));
+ if (!hierarchical_namespace_enabled) {
+ return Status::NotImplemented(
+ "Cannot create a directory without hierarchical namespace: ",
path.full_path);
+ }
+ auto directory_client =
datalake_service_client_->GetFileSystemClient(path.container)
+ .GetDirectoryClient(path.path_to_file);
Review Comment:
:+1: for @kou's the suggested re-name.
--
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]
