Skip to content
This repository has been archived by the owner on Nov 20, 2018. It is now read-only.

docs(features): Add S3 Transfer Acceleration to S3 feature #1627

Merged
merged 2 commits into from
Jul 28, 2016

Conversation

sohkai
Copy link
Contributor

@sohkai sohkai commented Jul 26, 2016

Brief description of the changes

Adds documentation changes to indicate support for S3 Transfer Acceleration (see #1556).

What browsers and operating systems have you tested these changes on?

N/A; docs change.

Are all automated tests passing?

N/A; docs change.

Is this pull request against develop or some other non-master branch?

No; docs change.


Fixes #1556. May also handle #1016 (just use the Transfer Accelerate option for now), although maybe in the last year Cloudfront has fixed their issues.

See additional comments based on changes.

We've been having success with this option on version 5.3, so I'd assume, based on the changes, that it should work for any version from at least 5.1 onwards. Maybe @DaveDeCaprio, @e-tip, or @ludofleury could chime in with which versions they've been using?

@CLAassistant
Copy link

CLAassistant commented Jul 26, 2016

CLA assistant check
All committers have signed the CLA.

headers to the request that is sent on to S3. An example of a CDN that should work fine is [fastly][fastly].
Ironically, an example of a CDN that does not appear to work is Amazon Cloudfront (see [issue #1016 in the
Fine Uploader GitHub issue tracker][issue1016]). If you'd like to use Amazon Cloudfront as a CDN for your
uploads, please use the [S3 Transfer Acceleration option][s3transferacceleration] instead.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@rnicholus
Copy link
Member

Thanks for the PR. I'll take a closer look when I get a chance and will provide feedback and suggestions to allow this to be merged.

@rnicholus
Copy link
Member

I think the best approach here is to completely remove the existing CloudFront-related documentation, and include a new sub-section under the main/existing "Uploading to S3 Through a CDN" section that talks about S3 Transfer Acceleration. Once #1016 has been verified, then we can add CF docs as part of closing that case.

@ludofleury
Copy link

I was using the latest version of FineUploader.

About #1016, I really think that this AWS accelerate feature remove the needs to do fancy thing with CloudFront "PUT". (On a side note, #1016 is actually working when configured without OAI).

But really, except if you upload more than 350 TB per month, I don't see a reason to not use accelerate instead of CloudFront.

@rnicholus
Copy link
Member

Thanks for your input @ludofleury. Perhaps the best course of action here is to:

  1. Close 5 - Support uploads to S3 via CloudFront distribution #1016 in favor of S3 Transfer Acceleration support.
  2. Remove mention of CloudFront in the docs, as I mentioned in my previous comment.

@ludofleury
Copy link

Yes, I totally agree on this, #1016 becomes obsolete (as the feature implemented).
I will try soon the accelerate feature with signed request (we have this requirement) and if I have any issue, I will get back there.

@sohkai
Copy link
Contributor Author

sohkai commented Jul 27, 2016

I've updated with the suggestions above, will squash after everything checks out.

@rnicholus
Copy link
Member

Looks pretty good to me. No need to worry about squashing. I can squash at merge time.

@rnicholus rnicholus merged commit 0dd21d3 into FineUploader:master Jul 28, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support S3 Transfer Acceleration
4 participants