feat(storage): integrate locationCredentialsProvider #13590
Conversation
israx
force-pushed
the
integrate-location-credentials-prov
branch
from
a4eec0d
to
1757c15
Compare
| export interface S3InternalConfig { | ||
| serviceOptions: S3ServiceOptions; | ||
| libraryOptions: S3LibraryOptions; | ||
| export interface S3InternalConfig extends S3ServiceOptions, S3LibraryOptions { |
There was a problem hiding this comment.
Why flatten this out further? It seems a little more organized to keep serviceOptions & libraryOptions in their own fields.
There was a problem hiding this comment.
This feedback came from @AllanZhengYP, the argument was that serviceOptions and libraryOptions are related to the configuration from the singleton and would be best just to pass the specific storage configuration. Open to discussion
| @@ -44,16 +68,194 @@ const createDefaultIdentityIdProvider = (amplify: AmplifyClassV6) => { | |||
| */ | |||
| export const createStorageConfiguration = ( | |||
| amplify: AmplifyClassV6, | |||
| apiInput: unknown, | |||
| permission: Permission, | |||
There was a problem hiding this comment.
I wonder if permission should just be optional so we don't need to pass in "dummy" values for our normal APIs.
There was a problem hiding this comment.
every Storage API needs to pass a permission to resolve the locationCredentialsProvider
There was a problem hiding this comment.
+1. I dont think we should pass in a value on all API that is not provided by the customer and we can not assume permission for them. we can keep it optional and throw an error from resolve for locationCreds if there is no permission in there. I think that will be better
Description of changes
Integrates the
locationCredentialsProviderinto the Storage APIsThe following changes will throw an exception,
locationCredentialsProvideris defined, andpathis not a string;bucketis an emptystringor is not defined;bucketandregionare not defined from the Amplify singleton or API inputIssue #, if available
Description of how you validated changes
Checklist
yarn testpassesChecklist for repo maintainers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.