Skip to content
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

commonPrefixes is only for delimiter request; check size and tier null #574

Merged
merged 2 commits into from
Nov 13, 2023
Merged

commonPrefixes is only for delimiter request; check size and tier null #574

merged 2 commits into from
Nov 13, 2023

Conversation

jixinchi
Copy link
Contributor

No description provided.

@gaul
Copy link
Owner

gaul commented Nov 13, 2023

These seem to make sense but could you explain the user symptoms more clearly, e.g., which provider does this currently fail on?

@jixinchi
Copy link
Contributor Author

jixinchi commented Nov 13, 2023

These seem to make sense but could you explain the user symptoms more clearly, e.g., which provider does this currently fail on?

I'm using filesystem as provider, and I have a problem of deleting a "directory" recursively by aws cli. The cli list files with Prefix and without Delimeter, and then delete each items in the Contents field of response. If there are empty directories below the directory to be deleted, these empty directories can not be deleted because they are not in the Contents filed.

There will be a jclouds PR to solve the problem that empty directories not in the Contents field. Here, if an empty directory item is in the Contents field, its size and tier may be null, so I check it here.

In addition, I checked aws doc, and I think commonPrefixes should be only for delimeter request, so I fixed it. But it won't cause failure.

@jixinchi
Copy link
Contributor Author

Sorry, there are something wrong in above expaination. If the request does not contains delimeter, it should be added to Contents. So I break the switch-case, not continue to next item

@gaul gaul merged commit 2a44bcd into gaul:master Nov 13, 2023
3 checks passed
@gaul
Copy link
Owner

gaul commented Nov 13, 2023

Thank you for your contribution and explanation @jixinchi!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants