-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Combining @uppy/aws-s3
and @uppy/aws-s3-multipart
#4218
Comments
Just ran into this, was expecting both plugins to "just" work alongside, then came to find a way to tell each one when to use, at the moment I think I can just dynamically This worked fine until I started using the dashboard. I tried to listen to the Another option could be having a way to set a plugin to In any case this feature would be highly appreciated, I really enjoy uppy so far, thanks a lot for this amazing project 🙏🏼 I think for now I can make my component decide based on |
That's an interesting idea, which could potentially be useful in more cases than just AWS. If this problem ever resurfaces in other part of the codebase we will definitely consider implementing that. When it comes to AWS though, I think it really makes more sense to combine both plugins so users don't have to duplicate their configuration and it would also simplify the maintenance to have one plugin instead of two. The only robust workaround for now is to have two Uppy instances, each with one plugin, but of course it's quickly become very hacky to make it seemless for end users, so you might be better off sticking to one or the other plugin for now until the combination work is done. |
Hi, may I know any updates on Golden Retrieval + S3 Multiparts implementation? |
@uppy/aws-s3
and @uppy/aws-s3-multpart
@uppy/aws-s3
and @uppy/aws-s3-multipart
This had been implemented in #4299. |
Initial checklist
Problem
We currently have two separate plugins for uploading to S3 (and S3-compatible services):
@uppy/aws-s3
and@uppy/aws-s3-multpart
. They have different use cases. The advantages of multipart uploads are:However, the downside is request overhead, as it needs to do creation, signing, and completion requests besides the upload requests. For example, if you are uploading files that are only a couple kilobytes with a 100ms roundtrip latency, you are spending 400ms on overhead and only a few milliseconds on uploading. This really adds up if you upload a lot of small files.
AWS, and generally the internet from what I found, tend to agree that you don't want to use multipart uploads for files under 100MB. But this sometimes puts users of our libraries in an awkward position, as their end users may not only upload very large files, or only small files. In this case a portion of their users get a subpar experience.
Solution
Combine the plugins, use multipart based on file size or a custom function that takes a file and returns for instance a boolean.
Scope
@uppy/aws-s3
has an optiongetResponseData()
for "use with almost S3-compatible storage solutions". I'd say the fact that we already support S3-compatible services (which we don't test against btw) is already enough and also trying to support "almost S3-compatible storage solutions" is too much.timeout
option. Wasn't even used in multipart plugin and doesn't seem to have a purpose anymore.getChunkSize
option. This should always be done by us, optimal part size is something we can calculate and letting the users do this is unnecessarily risky. See https://github.com/tus/tusd/blob/074d58300781cd68ed2f7768b8312ce666a6c43e/pkg/s3store/s3store.go#L943-L985.shouldUseMultipart: boolean | (file: UppyFile) => boolean
(or some other name).Considerations
@uppy/aws-s3
is also used by quite a few people to upload to S3-compatible providers, such as Google Cloud Storage. This is because the API surface is the same. But AFAIK that's not the case for multipart, or at the very least not the same amount of providers that offer regular S3 compatibility. This must be explained well in the docs.@uppy/aws-s3
and deprecating@uppy/aws-s3-multipart
.Alternatives
The text was updated successfully, but these errors were encountered: