Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(HNS folders): add new resources to support HNS folders #12101
base: main
Are you sure you want to change the base?
feat(HNS folders): add new resources to support HNS folders #12101
Changes from 60 commits
ad3cefc
b522bcd
de6f2ea
376eead
0646523
454d6d1
8eb8111
69d2966
ddd5b51
90599d5
9efe5a8
9b5195d
736407c
ad19c6a
1d6b68c
d81ca9e
e7b1ff6
06c78d3
a142e22
4650cd6
6098a8f
2a6d49d
6f767b7
6384419
ccebe43
1030aa1
f8786d8
8a86c0a
f20a949
a156f38
220bd1f
0756c9e
5c3b7cd
caac4ff
453aed3
7267414
92bd5fc
772d7b2
73a5bc8
cce471f
547875d
e6bbaac
d026479
d2f7d28
9221d77
2fa4f7a
f00da94
807016d
16f31c0
881bb8a
6c059e5
86bf3c0
f31d49a
3d83330
e5bd04f
62096c9
c5c00df
28b0e37
ce8deca
10f329f
0043ae2
e0c43cd
8775946
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry if I missed this: the bucket implementation appears to attempt the bucket deletion even if there is a listError, and then only surface the error if the deletion was not successful. Was there a reason we didn't do the same here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in bucket deletion, we are not attempting to delete the bucket if the listError occurs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do try to delete bucket even if the listError occurs,
Here we are only breaking object deletion loop in case of error. We try to delete bucket regardless of object deletion, Reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here I see there is no need to attempt to delete the folder,
foldersList, err := config.NewStorageClient(userAgent).Folders.List(bucket).Prefix(name).Do()
if len(foldersList.Items) == 1 || d.Get("force_destroy").(bool)
the API above call will return only the folder if it doesn't contain any sub folders if any sub folders exists it will return all the matching items so the logic is
if its the only folder exists we delete the folder irrespective of force destroy set to true or false,
if any subfolders exists and force_destroy set to true we delete the sub folders and the folder also, so in any case folder will be deleted
if subfolders exist and force destroy is false we throw an error in else case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this was added to the bucket implementation here: #2755. You can read the PR description, but it appears this was added to get around permissions (a user may be able to delete the bucket, but not list buckets).
I would defer to the two of you if the same issue will be present for folders.