felipecrv commented on code in PR #38708:
URL: https://github.com/apache/arrow/pull/38708#discussion_r1393558114
##########
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:
I can see why `path_to_file` is actually the name of the directory in this
context, but maybe a different name for this struct would make things less
confusing? This segment of filesystem paths is usually called "basename" [1].
[1] https://en.wikipedia.org/wiki/Basename
##########
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:
I spent quite some time trying to figure out what `if
(path.path_to_file.empty()) {` meant here.
`path.basename.empty()` would be more clear IMO.
cc @Tom-Newton
##########
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);
+ try {
+ auto response = directory_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 directory: ",
path.path_to_file, " (",
+ directory_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 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) {
+ return Status::NotImplemented(
+ "Cannot create a directory without hierarchical namespace: ",
path.full_path);
Review Comment:
I would write "...without hierarchical namespace *support*" so the user is
less likely to interpret this as some missing part of the path passed as
argument.
--
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]
