You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2024/04/08 09:43:50 UTC
[PR] GH-41034: WIP: [C++][FS][Azure] Adjust behavior against Azure for generic filesystem tests [arrow]
kou opened a new pull request, #41068:
URL: https://github.com/apache/arrow/pull/41068
### Rationale for this change
They are failing:
```text
[ FAILED ] 5 tests, listed below:
[ FAILED ] TestAzureHierarchicalNSGeneric.DeleteDir
[ FAILED ] TestAzureHierarchicalNSGeneric.DeleteDirContents
[ FAILED ] TestAzureHierarchicalNSGeneric.CopyFile
[ FAILED ] TestAzureHierarchicalNSGeneric.GetFileInfoSelector
[ FAILED ] TestAzureHierarchicalNSGeneric.SpecialChars
```
### What changes are included in this PR?
* `DeleteDir()`: Check not a directory case
* `DeleteDirContents()`: Check not a directory case
* `CopyFile()`: TODO
* `GetFileInfoSelector()`: TODO
* SpecialChars; TODO
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #41068:
URL: https://github.com/apache/arrow/pull/41068#issuecomment-2048619670
Thanks! I'll merge this.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #41068:
URL: https://github.com/apache/arrow/pull/41068#discussion_r1558759604
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -2168,13 +2203,6 @@ class AzureFileSystem::Impl {
// All the others either succeed or throw an exception.
DCHECK(response.Value.Deleted);
} catch (const Storage::StorageException& exception) {
- if (exception.ErrorCode == "FilesystemNotFound" ||
- exception.ErrorCode == "PathNotFound") {
- if (require_dir_to_exist) {
- return PathNotFound(location);
- }
- return Status::OK();
- }
Review Comment:
If we add a trailing slash, API returns an error...:
```text
'fs->DeleteDir("AB/CD")' failed with IOError: Failed to delete a directory: AB/CD: https://clearcodearrow.blob.core.windows.net/aeouspyxzsz16iliz71ya6yt5qrur6b9/AB/CD/ Azure Error: [InvalidUri] 400 The request URI is invalid.
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #41068:
URL: https://github.com/apache/arrow/pull/41068#discussion_r1558568410
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -1642,11 +1642,27 @@ class AzureFileSystem::Impl {
options.Prefix = {};
found = true; // Unless the container itself is not found later!
} else {
- options.Prefix = internal::EnsureTrailingSlash(base_location.path);
+ ARROW_ASSIGN_OR_RAISE(
+ auto prefix, AzureLocation::FromString(
+ std::string(internal::EnsureTrailingSlash(select.base_dir))));
+ ARROW_ASSIGN_OR_RAISE(auto info, GetFileInfo(container_client, prefix));
+ if (info.type() == FileType::NotFound) {
+ if (select.allow_not_found) {
+ return Status::OK();
+ } else {
+ return PathNotFound(base_location);
+ }
+ } else if (info.type() != FileType::Directory) {
+ return NotADir(base_location);
+ }
+ options.Prefix = prefix.path;
}
options.PageSizeHint = page_size_hint;
options.Include = Blobs::Models::ListBlobsIncludeFlags::Metadata;
+ auto adlfs_client = GetFileSystemClient(base_location.container);
+ ARROW_ASSIGN_OR_RAISE(auto hns_support, HierarchicalNamespaceSupport(adlfs_client));
Review Comment:
> I can understand what you say but the current implementation doesn't:
>
> GetFileInfoWithSelector() isn't dispatched based on HNS support:
But what I meant was changing/fixing `GetFileInfoWithSelector` to do that.
> Should we create GetFileInfoWithSelectorFromContainerOn{Container,FileSystem}()? But their names are too long...
ADLFS calls "filesystem" what "blobs" calls "container", so you replace the last word of the name:
```cpp
Status GetFileInfoWithSelectorFromContainer(const Blobs::BlobContainerClient& container_client, ...);
Status GetFileInfoWithSelecitonFromFilesystem(const DataLake::DataLakeFileSystemClient& adlfs_client, ...);
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #41068:
URL: https://github.com/apache/arrow/pull/41068#discussion_r1558277407
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -1642,11 +1642,27 @@ class AzureFileSystem::Impl {
options.Prefix = {};
found = true; // Unless the container itself is not found later!
} else {
- options.Prefix = internal::EnsureTrailingSlash(base_location.path);
+ ARROW_ASSIGN_OR_RAISE(
+ auto prefix, AzureLocation::FromString(
+ std::string(internal::EnsureTrailingSlash(select.base_dir))));
+ ARROW_ASSIGN_OR_RAISE(auto info, GetFileInfo(container_client, prefix));
+ if (info.type() == FileType::NotFound) {
+ if (select.allow_not_found) {
+ return Status::OK();
+ } else {
+ return PathNotFound(base_location);
+ }
+ } else if (info.type() != FileType::Directory) {
+ return NotADir(base_location);
+ }
+ options.Prefix = prefix.path;
}
options.PageSizeHint = page_size_hint;
options.Include = Blobs::Models::ListBlobsIncludeFlags::Metadata;
+ auto adlfs_client = GetFileSystemClient(base_location.container);
+ ARROW_ASSIGN_OR_RAISE(auto hns_support, HierarchicalNamespaceSupport(adlfs_client));
Review Comment:
I can understand what you say but the current implementation doesn't:
`GetFileInfoWithSelector()` isn't dispatched based on HNS support:
https://github.com/apache/arrow/blob/cd607d00b9e3c5f2ac2a2e373ba7571fd75fc725/cpp/src/arrow/filesystem/azurefs.cc#L2859-L2866
https://github.com/apache/arrow/blob/cd607d00b9e3c5f2ac2a2e373ba7571fd75fc725/cpp/src/arrow/filesystem/azurefs.cc#L1740-L1779
The `FromContainer` suffix doesn't mean only for no HNS support. We use `OnContainer`/`OnFileSystem` suffix for it. `FromContainer` just means that it's for one container and it doesn't work with multiple containers.
----
Should we create `GetFileInfoWithSelectorFromContainerOn{Container,FileSystem}()`? But their names are too long...
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #41068:
URL: https://github.com/apache/arrow/pull/41068#issuecomment-2048460039
@felipecrv Do you want to review this before we merge this? I want to add this to 16.0.0.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #41068:
URL: https://github.com/apache/arrow/pull/41068#discussion_r1557814378
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -1642,11 +1642,27 @@ class AzureFileSystem::Impl {
options.Prefix = {};
found = true; // Unless the container itself is not found later!
} else {
- options.Prefix = internal::EnsureTrailingSlash(base_location.path);
+ ARROW_ASSIGN_OR_RAISE(
+ auto prefix, AzureLocation::FromString(
+ std::string(internal::EnsureTrailingSlash(select.base_dir))));
+ ARROW_ASSIGN_OR_RAISE(auto info, GetFileInfo(container_client, prefix));
+ if (info.type() == FileType::NotFound) {
+ if (select.allow_not_found) {
+ return Status::OK();
+ } else {
+ return PathNotFound(base_location);
+ }
+ } else if (info.type() != FileType::Directory) {
+ return NotADir(base_location);
+ }
+ options.Prefix = prefix.path;
}
options.PageSizeHint = page_size_hint;
options.Include = Blobs::Models::ListBlobsIncludeFlags::Metadata;
+ auto adlfs_client = GetFileSystemClient(base_location.container);
+ ARROW_ASSIGN_OR_RAISE(auto hns_support, HierarchicalNamespaceSupport(adlfs_client));
Review Comment:
This fix is very confusing to me. This function is `GetFileInfoWithSelectorFromContainer`, it's meant to be called when HNS support has been detected as disabled.
The right fix (IMO) is to create `GetFileInfoWithSelectorFromFileSystem(adlfs_client, ...)` and then in `GetFileInfoWithSelector` dispatch to the different implementations according to the HNS support detection.
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -2168,13 +2203,6 @@ class AzureFileSystem::Impl {
// All the others either succeed or throw an exception.
DCHECK(response.Value.Deleted);
} catch (const Storage::StorageException& exception) {
- if (exception.ErrorCode == "FilesystemNotFound" ||
- exception.ErrorCode == "PathNotFound") {
- if (require_dir_to_exist) {
- return PathNotFound(location);
- }
- return Status::OK();
- }
Review Comment:
Given the TZ differences, I will give myself a chance of fixing these bugs to see if I can come up with something that doesn't introduce pessimistic round trips to the backend.
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -2168,13 +2203,6 @@ class AzureFileSystem::Impl {
// All the others either succeed or throw an exception.
DCHECK(response.Value.Deleted);
} catch (const Storage::StorageException& exception) {
- if (exception.ErrorCode == "FilesystemNotFound" ||
- exception.ErrorCode == "PathNotFound") {
- if (require_dir_to_exist) {
- return PathNotFound(location);
- }
- return Status::OK();
- }
Review Comment:
The fact that you check existence above doesn't mean these errors can't happen down here.
To use terms that @pitrou once shared with me [1], you're moving from EAFP [2] "easier to ask for permission" to LBYL [3] "look before you leap".
From [3]:
> ### LBYL
> Look before you leap. This coding style explicitly tests for pre-conditions before making calls or lookups. This style contrasts with the [EAFP](https://docs.python.org/3.5/glossary.html#term-eafp) approach and is characterized by the presence of many [if](https://docs.python.org/3.5/reference/compound_stmts.html#if) statements.
>
> In a multi-threaded environment, **the LBYL approach can risk introducing a race condition** between “the looking” and “the leaping”. For example, the code, if key in mapping: return mapping[key] can fail if another thread removes key from mapping after the test, but before the lookup. This issue can be solved with locks or by using the EAFP approach.
[1] https://devblogs.microsoft.com/python/idiomatic-python-eafp-versus-lbyl/
[2] https://docs.python.org/3.5/glossary.html#term-eafp
[3] https://docs.python.org/3.5/glossary.html#term-lbyl
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: WIP: [C++][FS][Azure] Adjust behavior against Azure for generic filesystem tests [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #41068:
URL: https://github.com/apache/arrow/pull/41068#issuecomment-2044416691
@felipecrv It seems that the current `CopyFile` implementation doesn't work with Azure hierarchical namespace support. I'll split it to a separated issue.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #41068:
URL: https://github.com/apache/arrow/pull/41068#discussion_r1558822366
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -1642,11 +1642,27 @@ class AzureFileSystem::Impl {
options.Prefix = {};
found = true; // Unless the container itself is not found later!
} else {
- options.Prefix = internal::EnsureTrailingSlash(base_location.path);
+ ARROW_ASSIGN_OR_RAISE(
+ auto prefix, AzureLocation::FromString(
+ std::string(internal::EnsureTrailingSlash(select.base_dir))));
+ ARROW_ASSIGN_OR_RAISE(auto info, GetFileInfo(container_client, prefix));
+ if (info.type() == FileType::NotFound) {
+ if (select.allow_not_found) {
+ return Status::OK();
+ } else {
+ return PathNotFound(base_location);
+ }
+ } else if (info.type() != FileType::Directory) {
+ return NotADir(base_location);
+ }
+ options.Prefix = prefix.path;
}
options.PageSizeHint = page_size_hint;
options.Include = Blobs::Models::ListBlobsIncludeFlags::Metadata;
+ auto adlfs_client = GetFileSystemClient(base_location.container);
+ ARROW_ASSIGN_OR_RAISE(auto hns_support, HierarchicalNamespaceSupport(adlfs_client));
Review Comment:
Implemented.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: WIP: [C++][FS][Azure] Adjust behavior against Azure for generic filesystem tests [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41068:
URL: https://github.com/apache/arrow/pull/41068#issuecomment-2042312547
:warning: GitHub issue #41034 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #41068:
URL: https://github.com/apache/arrow/pull/41068
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #41068:
URL: https://github.com/apache/arrow/pull/41068#discussion_r1558301639
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -2168,13 +2203,6 @@ class AzureFileSystem::Impl {
// All the others either succeed or throw an exception.
DCHECK(response.Value.Deleted);
} catch (const Storage::StorageException& exception) {
- if (exception.ErrorCode == "FilesystemNotFound" ||
- exception.ErrorCode == "PathNotFound") {
- if (require_dir_to_exist) {
- return PathNotFound(location);
- }
- return Status::OK();
- }
Review Comment:
Can we block changing the target location from other connections by using `lease_id` for `GetFileInfo()` too?
```diff
diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc
index df6db3d959..67232b3156 100644
--- a/cpp/src/arrow/filesystem/azurefs.cc
+++ b/cpp/src/arrow/filesystem/azurefs.cc
@@ -2181,7 +2181,7 @@ class AzureFileSystem::Impl {
Azure::Nullable<std::string> lease_id = {}) {
DCHECK(!location.container.empty());
DCHECK(!location.path.empty());
- ARROW_ASSIGN_OR_RAISE(auto file_info, GetFileInfo(adlfs_client, location));
+ ARROW_ASSIGN_OR_RAISE(auto file_info, GetFileInfo(adlfs_client, location, lease_id));
if (file_info.type() == FileType::NotFound) {
if (require_dir_to_exist) {
return PathNotFound(location);
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #41068:
URL: https://github.com/apache/arrow/pull/41068#discussion_r1558574621
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -2168,13 +2203,6 @@ class AzureFileSystem::Impl {
// All the others either succeed or throw an exception.
DCHECK(response.Value.Deleted);
} catch (const Storage::StorageException& exception) {
- if (exception.ErrorCode == "FilesystemNotFound" ||
- exception.ErrorCode == "PathNotFound") {
- if (require_dir_to_exist) {
- return PathNotFound(location);
- }
- return Status::OK();
- }
Review Comment:
> But DeleteRecursive()/DeleteEmpty() can delete both of file and directory. See also: https://learn.microsoft.com/en-us/rest/api/storageservices/datalakestoragegen2/path/delete?view=rest-storageservices-datalakestoragegen2-2019-12-12
This scenario sounds familiar. 🤔 Does that happen even if you append a trailing slash to the path?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #41068:
URL: https://github.com/apache/arrow/pull/41068#issuecomment-2048923675
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 06d44951736b5a6e5836f2ea2fc988d935ea7f32.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/23691622464) has more details. It also includes information about 12 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #41068:
URL: https://github.com/apache/arrow/pull/41068#discussion_r1558296508
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -2168,13 +2203,6 @@ class AzureFileSystem::Impl {
// All the others either succeed or throw an exception.
DCHECK(response.Value.Deleted);
} catch (const Storage::StorageException& exception) {
- if (exception.ErrorCode == "FilesystemNotFound" ||
- exception.ErrorCode == "PathNotFound") {
- if (require_dir_to_exist) {
- return PathNotFound(location);
- }
- return Status::OK();
- }
Review Comment:
OK. I'll revert this for now.
You can introduce better approach later if you can find it.
FYI: We want to report a `NotADir()` for file location. But `DeleteRecursive()`/`DeleteEmpty()` can delete both of file and directory. See also: https://learn.microsoft.com/en-us/rest/api/storageservices/datalakestoragegen2/path/delete?view=rest-storageservices-datalakestoragegen2-2019-12-12
> Delete the file or directory.
https://github.com/apache/arrow/blob/cd607d00b9e3c5f2ac2a2e373ba7571fd75fc725/cpp/src/arrow/filesystem/test_util.cc#L274-L276 is a test for this case.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-41034: [C++][FS][Azure] Adjust DeleteDir/DeleteDirContents/GetFileInfoSelector behaviors against Azure for generic filesystem tests [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #41068:
URL: https://github.com/apache/arrow/pull/41068#discussion_r1558760826
##########
cpp/src/arrow/filesystem/azurefs.cc:
##########
@@ -1642,11 +1642,27 @@ class AzureFileSystem::Impl {
options.Prefix = {};
found = true; // Unless the container itself is not found later!
} else {
- options.Prefix = internal::EnsureTrailingSlash(base_location.path);
+ ARROW_ASSIGN_OR_RAISE(
+ auto prefix, AzureLocation::FromString(
+ std::string(internal::EnsureTrailingSlash(select.base_dir))));
+ ARROW_ASSIGN_OR_RAISE(auto info, GetFileInfo(container_client, prefix));
+ if (info.type() == FileType::NotFound) {
+ if (select.allow_not_found) {
+ return Status::OK();
+ } else {
+ return PathNotFound(base_location);
+ }
+ } else if (info.type() != FileType::Directory) {
+ return NotADir(base_location);
+ }
+ options.Prefix = prefix.path;
}
options.PageSizeHint = page_size_hint;
options.Include = Blobs::Models::ListBlobsIncludeFlags::Metadata;
+ auto adlfs_client = GetFileSystemClient(base_location.container);
+ ARROW_ASSIGN_OR_RAISE(auto hns_support, HierarchicalNamespaceSupport(adlfs_client));
Review Comment:
> ADLFS calls "filesystem" what "blobs" calls "container"
Wow! I didn't notice it!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org