-
Notifications
You must be signed in to change notification settings - Fork 119
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(storage): update AWSS3StoragePlugin to support multiple buckets #2895
feat(storage): update AWSS3StoragePlugin to support multiple buckets #2895
Conversation
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.
There are a few options types in package com.amplifyframework.storage.options
that still need updated.
Main concern at the moment is the storage service lookup. I may have additional comments on the AWSS3StoragePlugin once the changes are considered.
If at all possible, can you try to revert the unnecessary formatting changes so its easier to see what has changed due to feature implementation.
core/src/test/java/com/amplifyframework/core/configuration/AmplifyOutputsDataTest.kt
Outdated
Show resolved
Hide resolved
core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt
Outdated
Show resolved
Hide resolved
aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java
Show resolved
Hide resolved
aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java
Outdated
Show resolved
Hide resolved
I've resolved the review comments. I've intentionally excluded the other option types in package |
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.
Thanks for addressing. I was not aware about bucket names being unique regardless of region
Multi-bucket support part 1
Issue #, if available:
Description of changes:
This the first part of a series of changes/pull request to support multi-bucket in the S3 Storage Plugin. The series of changes will be reviewed and merged into a feature branch and combined into a single pull request to be reviewed and merged into
main
branch.Changes in this pull request includes:
AmplifyOutputsData
to support multi-bucket configurationStorageOption
to specify S3 bucketAWSS3StoragePlugin
to support interfacing with unique bucket/region by creating and maintaining multipleAWSS3StorageService
instances. Creating multipleAWSS3StorageService
at the plugin level is the least invasive change to support multiple buckets/regions; the alternative is to create and maintain multipleS3Client
at the operation layer and result in updating every operations in the Storage plugin.getUrl
API to support multiple buckets. The rest of the Storage plugin API will be updated in subsequent pull requests.Integration tests will be added in a separate pull request.
How did you test these changes?
Documentation update required?
General Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.