fix: managed upload resolves config repeatedly #3700
Merged
+28
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AllanZhengYP
force-pushed
the
fix-upload-creds
branch
from
f84ff26 to
839a03c
Compare
trivikr
reviewed
Apr 7, 2021
Previously the managed upload creates new S3 clients with the constructor parameters of the S3 client used to create the managed upload. That results in that the newly created S3 client needs to resolve every config, including the credentials over and over again. This issue was introduced by aws#3109 to make sure the internal S3 client inside of managed upload has the same constructor parameters of the S3 client used to create the managed upload. So that the logic relies on the constructor parameters like access point and endpoint discovery works properly. This change supply the resolved client config to create the managed upload internal S3 client so configs don't need to be resolved again, like credentials. Meanwhile, we explicitly set the constructor parameter to be the same as that of S3 client used to create the managed upload.
Co-authored-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
AllanZhengYP
force-pushed
the
fix-upload-creds
branch
from
839a03c to
ec2df2c
Compare
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
trivikr
approved these changes
Apr 8, 2021
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the managed upload creates new S3 clients with the
constructor parameters of the S3 client used to create the managed
upload. That results in that the newly created S3 client needs to
resolve every config, including the credentials over and over
again.
This issue was introduced by #3109 to make sure the internal S3
client inside of managed upload has the same constructor parameters
of the S3 client used to create the managed upload. So that the
logic relies on the constructor parameters like access point
and endpoint discovery works properly.
This change supply the resolved client config to create the
managed upload internal S3 client so configs don't need to be
resolved again, like credentials. Meanwhile, we explicitly set
the constructor parameter to be the same as that of S3 client
used to create the managed upload.
Resolves: #3481
Checklist
npm run testpassesnpm run add-change