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

feat(storage): update s3 storage service to support multiple s3 client for multi-bucket support #5493

Merged
merged 2 commits into from
Sep 25, 2024

Conversation

NikaHsn
Copy link
Member

@NikaHsn NikaHsn commented Sep 24, 2024

Issue #, if available:

Description of changes:

  • update storage bucket from outputs to get the bucket info from storage outputs
  • update storage service to create and cache s3 clients based on the bucket info
  • update storage service to move sigv4 signer fields to getUrl method as they are used only by this method.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@NikaHsn NikaHsn requested a review from a team as a code owner September 24, 2024 21:35
Comment on lines 29 to 43
orElse: () => throw const InvalidStorageBucketException(
'Unable to lookup bucket from provided name in Amplify Outputs file.',
recoverySuggestion: 'Make sure Amplify Outputs file has the specified '
'bucket configuration.',
),
);
if (bucket == null) {
throw const InvalidStorageBucketException(
'Amplify Outputs storage configuration does not have buckets specified.',
recoverySuggestion:
'Make sure Amplify Outputs file has storage configuration with '
'buckets specified.',
);
}
return BucketInfo(bucketName: bucket.bucketName, region: bucket.awsRegion);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit suggestion: Perform the null check before accessing storageOutputs.buckets.singleWhere. I initially overlooked the optional chaining, which made orElse appear redundant upon first glance.

Comment on lines +5 to +6
@internal
class S3ClientInfo {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add docs comment

@NikaHsn NikaHsn merged commit d68bb03 into multi-bucket Sep 25, 2024
155 checks passed
@Equartey Equartey deleted the feat/s3-storage-service-multibucket-support branch September 26, 2024 18:19
ekjotmultani pushed a commit that referenced this pull request Oct 18, 2024
NikaHsn added a commit that referenced this pull request Oct 23, 2024
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.

4 participants