You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "felipecrv (via GitHub)" <gi...@apache.org> on 2024/03/27 14:13:25 UTC
[PR] MINOR: [C++][Azure][FS] Document some limitations and atomicity guarantees [arrow]
felipecrv opened a new pull request, #40838:
URL: https://github.com/apache/arrow/pull/40838
### Rationale for this change
Documenting some details of the behavior of destructive filesystem operations.
### What changes are included in this PR?
Only docstring changes.
### Are these changes tested?
N/A.
--
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] MINOR: [C++][Azure][FS] Document some limitations and atomicity guarantees [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #40838:
URL: https://github.com/apache/arrow/pull/40838#discussion_r1542024030
##########
cpp/src/arrow/filesystem/azurefs.h:
##########
@@ -264,15 +264,35 @@ class ARROW_EXPORT AzureFileSystem : public FileSystem {
Status CreateDir(const std::string& path, bool recursive) override;
+ /// \brief Delete a directory and its contents recursively.
+ ///
+ /// Atomicity is guaranteed only on Hierarchical Namespace Storage accounts.
Status DeleteDir(const std::string& path) override;
+ /// \brief Non-atomically deletes the contents of a directory.
+ ///
+ /// This function can return a bad Status after only partially deleting the
+ /// contents of the directory.
Status DeleteDirContents(const std::string& path, bool missing_dir_ok) override;
+ /// \brief Deletion of all the containers in the storage account (not
+ /// implemented for safety reasons).
+ ///
+ /// \return Status::NotImplemented
Status DeleteRootDirContents() override;
+ /// \brief Deletes a file.
+ ///
+ /// Supported on both flat namespace and Hierarchical Namespace storage
+ /// accounts. A check is made to guarantee the parent directory doesn't
+ /// disappear after the blob is deleted and while this operation is running,
Review Comment:
Re-wording suggestion? My intention is being concise, but yeah, none of this applies to HNS accounts.
--
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] MINOR: [C++][Azure][FS] Document some limitations and atomicity guarantees [arrow]
Posted by "Tom-Newton (via GitHub)" <gi...@apache.org>.
Tom-Newton commented on code in PR #40838:
URL: https://github.com/apache/arrow/pull/40838#discussion_r1541935615
##########
cpp/src/arrow/filesystem/azurefs.h:
##########
@@ -264,15 +264,35 @@ class ARROW_EXPORT AzureFileSystem : public FileSystem {
Status CreateDir(const std::string& path, bool recursive) override;
+ /// \brief Delete a directory and its contents recursively.
+ ///
+ /// Atomicity is guaranteed only on Hierarchical Namespace Storage accounts.
Status DeleteDir(const std::string& path) override;
+ /// \brief Non-atomically deletes the contents of a directory.
+ ///
+ /// This function can return a bad Status after only partially deleting the
+ /// contents of the directory.
Status DeleteDirContents(const std::string& path, bool missing_dir_ok) override;
+ /// \brief Deletion of all the containers in the storage account (not
+ /// implemented for safety reasons).
+ ///
+ /// \return Status::NotImplemented
Status DeleteRootDirContents() override;
+ /// \brief Deletes a file.
+ ///
+ /// Supported on both flat namespace and Hierarchical Namespace storage
+ /// accounts. A check is made to guarantee the parent directory doesn't
+ /// disappear after the blob is deleted and while this operation is running,
Review Comment:
Is this referring to flat namespace accounts only? I can't imagine a hierarchical namespace directory disappearing when a file is deleted.
--
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] MINOR: [C++][Azure][FS] Document some limitations and atomicity guarantees [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #40838:
URL: https://github.com/apache/arrow/pull/40838#issuecomment-2022879292
@Tom-Newton
--
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] MINOR: [C++][Azure][FS] Document some limitations and atomicity guarantees [arrow]
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv merged PR #40838:
URL: https://github.com/apache/arrow/pull/40838
--
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] MINOR: [C++][Azure][FS] Document some limitations and atomicity guarantees [arrow]
Posted by "Tom-Newton (via GitHub)" <gi...@apache.org>.
Tom-Newton commented on code in PR #40838:
URL: https://github.com/apache/arrow/pull/40838#discussion_r1542039623
##########
cpp/src/arrow/filesystem/azurefs.h:
##########
@@ -264,15 +264,35 @@ class ARROW_EXPORT AzureFileSystem : public FileSystem {
Status CreateDir(const std::string& path, bool recursive) override;
+ /// \brief Delete a directory and its contents recursively.
+ ///
+ /// Atomicity is guaranteed only on Hierarchical Namespace Storage accounts.
Status DeleteDir(const std::string& path) override;
+ /// \brief Non-atomically deletes the contents of a directory.
+ ///
+ /// This function can return a bad Status after only partially deleting the
+ /// contents of the directory.
Status DeleteDirContents(const std::string& path, bool missing_dir_ok) override;
+ /// \brief Deletion of all the containers in the storage account (not
+ /// implemented for safety reasons).
+ ///
+ /// \return Status::NotImplemented
Status DeleteRootDirContents() override;
+ /// \brief Deletes a file.
+ ///
+ /// Supported on both flat namespace and Hierarchical Namespace storage
+ /// accounts. A check is made to guarantee the parent directory doesn't
+ /// disappear after the blob is deleted and while this operation is running,
Review Comment:
Less concise but I would go for
```suggestion
/// accounts. On flat namespace storage accounts a check is made to guarantee the parent directory doesn't
/// disappear after the blob is deleted and while this operation is running,
```
I don't feel strongly
--
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] MINOR: [C++][Azure][FS] Document some limitations and atomicity guarantees [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #40838:
URL: https://github.com/apache/arrow/pull/40838#issuecomment-2026678228
After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit edf7e57127766e0e2aa7d14db12d3d3f5f12ecbe.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/23228722002) has more details.
--
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