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

Simplify S3ConnectionTrait to defaultProvider plus option #27493

Merged
merged 1 commit into from
Dec 21, 2021
Merged

Simplify S3ConnectionTrait to defaultProvider plus option #27493

merged 1 commit into from
Dec 21, 2021

Conversation

cuppett
Copy link
Contributor

@cuppett cuppett commented Jun 13, 2021

When we initially added the EC2 and ECS IAM role support in #24700,
we had to use a workaround by explicitly ordering the various providers
due to an inconsistency in the AWS SDK for PHP. We submitted a PR there
to get that squared away. Now, we've consumed that version upstream
for the SDK and can update our code here to be the most concise version
as well as position ourselves to pick up new methods as those become
available and prevalent in AWS (for acquiring credentials).

See also: #24700 (comment)
See also: aws/aws-sdk-php#2172

@solracsf solracsf requested a review from rullzer July 1, 2021 11:32
@solracsf solracsf added the 3. to review Waiting for reviews label Jul 1, 2021
@skjnldsv skjnldsv requested a review from a team August 18, 2021 12:18
@skjnldsv

This comment has been minimized.

@cuppett

This comment has been minimized.

lib/private/Files/ObjectStore/S3ConnectionTrait.php Outdated Show resolved Hide resolved
@skjnldsv
Copy link
Member

/rebase

No problem. Done. Thanks!

Just as a clarification, we have a bot (that failed) for that 😉
That was a command for it. Thanks for the manual rebase 🤗

@skjnldsv skjnldsv requested review from icewind1991 and a team August 19, 2021 08:28
@skjnldsv skjnldsv added this to the Nextcloud 23 milestone Aug 19, 2021
@cuppett cuppett dismissed juliushaertl’s stale review September 18, 2021 13:46

Resolved this. Thanks! Can this be merged now?

@skjnldsv skjnldsv mentioned this pull request Oct 13, 2021
@skjnldsv skjnldsv added the bug label Oct 22, 2021
This was referenced Oct 25, 2021
@skjnldsv skjnldsv modified the milestones: Nextcloud 23, Nextcloud 24 Nov 1, 2021
When we initially added the EC2 and ECS IAM role support in #24700,
we had to use a workaround by explicitly ordering the various providers
due to an inconsistency in the AWS SDK for PHP. We submitted a PR there
to get that squared away. Now, we've consumed that version upstream
for the SDK and can update our code here to be the most concise version
as well as position ourselves to pick up new methods as those become
available and prevalent in AWS (for acquiring credentials).

See also: #24700 (comment)
See also: aws/aws-sdk-php#2172

Signed-off-by: Stephen Cuppett <steve@cuppett.com>
@cuppett
Copy link
Contributor Author

cuppett commented Nov 22, 2021

Went ahead and rebased this on master

@juliushaertl juliushaertl merged commit 1acfbd0 into nextcloud:master Dec 21, 2021
@cuppett cuppett deleted the cuppett/simplify-aws-credential-provider branch December 21, 2021 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants