-
Notifications
You must be signed in to change notification settings - Fork 25
Get bucket name directly from path to avoid calling 'listBuckets' #891
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
Conversation
@steve-todorov did you already find the time to look into this? |
@oeph Apologies was very busy the past couple of months and dropped the ball on this. I'll be doing some dependency updates this week so I'll check this one too. |
When using S3FileStore::getBucket the AWS user needs the permission 's3:ListAllMyBuckets'. In S3FileSystemProvider::createDirectory it is checked if a bucket exists, but this uses S3FileStore::getBucket thus needing the additional permission, even if this shouldn't be the case. This commit adds S3FileStore::hasBucket to avoid the mentioned method call in S3FileSystemProvider.
@steve-todorov any updates on this? |
@oeph i was able to do some maintenance tasks last week and had a quick peek at this PR too. The tests are still failing which means there's a problem with the proposed change. I wasn't able to go through a full debug session to pinpoint the exact cause but might be related to the FileStores where the bucket is also needed or a related part. |
@kxm-kstlr Thanks for raising the PR and for your patience. I finally had some time to look into this in more depth. Long story short -- the At the time of writing this, there is simply no way to fetch this other than pulling it from the This is partially why the tests in the CI were failing as well. Unfortunately we will have to close this PR and the task as "wont fix", simply because of AWS S3 Rest API limitations. P.S. We're open to revisit this should the AWS S3 API change or if another way becomes available. |
Pull Request Description
This pull request closes #890 .
Acceptance Test
gradle clean build
still works.Questions
Does this pull request break backward compatibility?
Does this pull request require other pull requests to be merged first?
Does this require an update of the documentation?