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.

Sign up for GitHub

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

Jump to bottom

feat: support "s3-accelerate" endpoint #44496

Merged
merged 1 commit into from
Apr 26, 2024
Merged

feat: support "s3-accelerate" endpoint #44496

merged 1 commit into from
Apr 26, 2024

Conversation

icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Mar 26, 2024 •
edited by susnux
Loading

When the S3 hostname is set to s3-accelerate.amazonaws.com, enable the use_accelerate_endpoint.

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Mar 26, 2024
@icewind1991 icewind1991 added this to the Nextcloud 29 milestone Mar 26, 2024
@icewind1991 icewind1991 requested review from a team, ArtificialOwl, nfebe and sorbaugh and removed request for a team March 26, 2024 18:00
@joshtrichards
Copy link
Member

Relevant issue: #42675

@Altahrim Altahrim mentioned this pull request Mar 26, 2024
Merged
@solracsf
Copy link
Member

Will this handle also:

bucketname.s3-accelerate.amazonaws.com
bucketname.s3-accelerate.dualstack.amazonaws.com

?

@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
Merged
81 tasks
@sorbaugh
Copy link
Contributor

sorbaugh commented Apr 2, 2024 •
edited
Loading

Will this handle also:

bucketname.s3-accelerate.amazonaws.com
bucketname.s3-accelerate.dualstack.amazonaws.com

?

Hi solracsf, this PR doesn't handle dualstack atm

This was referenced Apr 4, 2024
Merged
Merged
@blizzz blizzz modified the milestones: Nextcloud 29, Nextcloud 30 Apr 8, 2024
@icewind1991
Copy link
Member Author

added dualstack support

@sorbaugh
Copy link
Contributor

added dualstack support

🤩

@icewind1991
feat: add option to enable s3 acceleration
40d9a3f 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 merged commit a2fefbc into master Apr 26, 2024
157 checks passed
@icewind1991 icewind1991 deleted the feat/s3-accelerate branch April 26, 2024 13:43
@icewind1991
Copy link
Member Author

/backport to stable29

@backportbot backportbot bot mentioned this pull request Apr 26, 2024
Merged
@icewind1991
Copy link
Member Author

/backport to stable28

@backportbot backportbot bot mentioned this pull request Apr 26, 2024
Merged
@icewind1991
Copy link
Member Author

/backport to stable27

@backportbot backportbot bot mentioned this pull request Apr 26, 2024
Merged
joshtrichards added a commit that referenced this pull request Jun 3, 2024
@joshtrichards
fix(ObjectStore): Fix regression when hostname is empty
168d3a9 
Fixes #45637 

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards mentioned this pull request Jun 3, 2024
Merged
5 tasks
joshtrichards added a commit that referenced this pull request Jun 3, 2024
@joshtrichards
fix(ObjectStore): Fix regression when hostname is empty
5defd2a 
Fixes #45637

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
joshtrichards added a commit that referenced this pull request Jun 3, 2024
@joshtrichards
fix(ObjectStore): Fix regression when hostname is empty
cb7bbf6 
Fixes #45637

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
joshtrichards added a commit that referenced this pull request Jun 4, 2024
@joshtrichards
fix(ObjectStore): Fix regression when hostname is empty
b4e8160 
Fixes #45637

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
kesselb pushed a commit that referenced this pull request Jun 12, 2024
@joshtrichards @kesselb
fix(ObjectStore): Fix regression when hostname is empty
a7bf2ba 
Fixes #45637

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
joshtrichards added a commit that referenced this pull request Jun 12, 2024
@joshtrichards
fix(ObjectStore): handle empty S3 hostname
6388614 
Fixes #45637

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
backportbot bot pushed a commit that referenced this pull request Jun 12, 2024
@joshtrichards @backportbot
fix(ObjectStore): handle empty S3 hostname
3fde98d 
Fixes #45637

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
backportbot bot pushed a commit that referenced this pull request Jun 12, 2024
@joshtrichards @backportbot
fix(ObjectStore): handle empty S3 hostname
6cf67ab 
Fixes #45637

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
backportbot bot pushed a commit that referenced this pull request Jun 12, 2024
@joshtrichards @backportbot
fix(ObjectStore): handle empty S3 hostname
77c280a 
Fixes #45637

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
susnux pushed a commit that referenced this pull request Jun 13, 2024
@joshtrichards @susnux
fix(ObjectStore): handle empty S3 hostname
fcdf453 
Fixes #45637

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
kesselb pushed a commit that referenced this pull request Jun 13, 2024
@joshtrichards @kesselb
fix(ObjectStore): handle empty S3 hostname
d285ade 
Fixes #45637

The support for s3-accelerate added in #44496 introduced a regression in AWS S3 environments when `hostname` is blank (which is a valid configuration w/ AWS since the hostname gets auto-generated).

Signed-off-by: Josh <josh.t.richards@gmail.com>
@blizzz blizzz mentioned this pull request Jul 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sorbaugh sorbaugh sorbaugh approved these changes

@artonge artonge artonge approved these changes

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl was automatically assigned from nextcloud/server-backend

@nfebe nfebe Awaiting requested review from nfebe nfebe was automatically assigned from nextcloud/server-backend

Assignees
No one assigned
Labels
3. to review Waiting for reviews backport-request
Projects
None yet
Milestone
Nextcloud 30
Development

Successfully merging this pull request may close these issues.

Current S3 Implementation lacks support for different endpoints (e.g. S3 Transfer Acceleration)
6 participants
@icewind1991 @joshtrichards @solracsf @sorbaugh @artonge @blizzz