feat(storage): add sample for hierarchical namespace recursive delete - #16402
feat(storage): add sample for hierarchical namespace recursive delete#16402nidhiii-27 wants to merge 4 commits into
Conversation
Adds a C++ code sample demonstrating hierarchical namespace recursive folder delete. Fixes: b/530058946 [Generated-by: AI]
There was a problem hiding this comment.
Code Review
This pull request adds a new sample function DeleteFolderRecursive to demonstrate and test recursive folder deletion in the Storage Control service. It also updates the AutoRun integration test and the command-line entry points to include this new functionality. Feedback on the code suggests explicitly declaring the return type instead of using auto when calling DeleteFolderRecursive().get(), in accordance with the repository's style guide regarding obscured return types.
| std::string const& bucket_name, std::string const& folder_id) { | ||
| auto const name = std::string{"projects/_/buckets/"} + bucket_name + | ||
| "/folders/" + folder_id; | ||
| auto status = client.DeleteFolderRecursive(name).get(); |
There was a problem hiding this comment.
According to the repository style guide, using auto is disallowed when it obscures StatusOr<T> or other return types. Please explicitly specify the return type google::cloud::StatusOr<google::protobuf::Empty>.
google::cloud::StatusOr<google::protobuf::Empty> status =
client.DeleteFolderRecursive(name).get();References
- Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. (link)
There was a problem hiding this comment.
Done
Co-authored by AI Agent
[Generated-by: AI]
[Generated-by: AI]
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16402 +/- ##
==========================================
- Coverage 92.26% 92.24% -0.03%
==========================================
Files 2246 2246
Lines 212121 212121
==========================================
- Hits 195707 195661 -46
- Misses 16414 16460 +46 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| auto const parent = std::string{"projects/_/buckets/"} + bucket_name; | ||
| for (auto folder : client.ListFolders(parent)) { | ||
| if (!folder) throw std::move(folder).status(); | ||
| if (!std::regex_match(folder->name(), re)) continue; |
There was a problem hiding this comment.
IIUC, folder->name() returns the full resource name (e.g. projects/_/buckets//folders/<folder_id>). Because std::regex_match requires the full string to match the pattern, matching against prefix + ... will fail.
Here and below.
|
|
||
| // Verify deletion by checking that getting the parent folder fails with | ||
| // NOT_FOUND. | ||
| try { |
There was a problem hiding this comment.
Let's verify that recursive_child_id was also deleted and returns kNotFound.
|
|
||
| auto const recursive_parent_id = | ||
| prefix + "-recursive-" + | ||
| google::cloud::internal::Sample(generator, 16, |
There was a problem hiding this comment.
nit: folder_id and dest_folder_id in this file use 32 random characters (Sample(generator, 32, ...)). Using 32 for recursive_parent_id as well keeps ID lengths uniform and avoids needing the {16,32} range in the RemoveStaleFolders regex.
| void DeleteFolderRecursive( | ||
| google::cloud::storagecontrol_v2::StorageControlClient client, | ||
| std::vector<std::string> const& argv) { | ||
| // [START storage_control_delete_folder_recursive] |
There was a problem hiding this comment.
To make this sample consistent with other snippets in this file:
- Use auto for the LRO future result.
- Use if (!result) instead of if (!status.ok()) to match how StatusOr is checked elsewhere in this file.
- Add an explanatory comment about .get() blocking on the LRO, matching RenameFolder.
Draft PR for hierarchical namespace recursive delete folder sample.
[Generated-by: AI]