@uppy/aws-s3: add shouldUseMultipart option
#4299
Merged
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
aduh95
force-pushed
the
aws-shouldUseMultipart
branch
from
ecee9f6
to
7e4608b
Compare
aduh95
added
the
safe to test
github-actions
bot
removed
pending end-to-end tests
safe to test
Murderlon
requested changes
Apr 28, 2023
mifi
reviewed
May 2, 2023
|
We discussed the increase in size with Antoine and decided it's probably worth it (30kb minified), but I missed the point that per-file conditions won't be applied here indeed? Then it's probably not worth it. So: we tell users who want new conditions to use |
arturi
force-pushed
the
aws-shouldUseMultipart
branch
from
228101f
to
7e4608b
Compare
|
Yes this temporary solution here only creates more confusion. Both plugins have a |
|
Both options behave exactly the same: either per file or as a permanent switch. When provided, aws-s3 plugin would behave exactly as if it was aws-s3-multipart, allowing folks to start using the new behavior without having to rewrite all their imports. |
|
Ah right, it returns the new Multipart, but it then gets the same option, so it does pass the per-file conditions to it. Sorry, it should be clear enough by now, but even after working with it and testing both plugins with/without options I still was confused in my comment above. |
Murderlon
approved these changes
May 3, 2023
There was a problem hiding this comment.
Unless we are doing a major soon, I don't think it makes sense to make this package quite a bit heavier if the switching functionality is already in multipart. That would leave users with a heavier package with no gains for them. But if majority agrees then code LGTM
|
FWIW, I agree there’s no rush, we probably want to wait a bit to be sure the feature is stable, and to have a blog post ready to explain what are our plans for the S3 plugin(s). |
aduh95
added
the
safe to test
github-actions
bot
removed
pending end-to-end tests
safe to test
Merged
github-actions bot
added a commit
that referenced
this pull request
Jun 19, 2023
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/aws-s3 | 3.2.0 | @uppy/status-bar | 3.2.0 | | @uppy/aws-s3-multipart | 3.4.0 | @uppy/transloadit | 3.1.6 | | @uppy/companion | 4.5.1 | @uppy/tus | 3.1.1 | | @uppy/core | 3.2.1 | @uppy/url | 3.3.2 | | @uppy/dashboard | 3.4.1 | @uppy/utils | 5.4.0 | | @uppy/golden-retriever | 3.0.4 | @uppy/xhr-upload | 3.3.0 | | @uppy/locales | 3.2.2 | uppy | 3.10.0 | | @uppy/provider-views | 3.3.1 | | | - @uppy/aws-s3-multipart: fix the chunk size calculation (Antoine du Hamel / #4508) - @uppy/aws-s3: add `shouldUseMultipart` option (Antoine du Hamel / #4299) - @uppy/companion: switch from aws-sdk v2 to @aws-sdk/* (v3) (Scott Bessler / #4285) - @uppy/companion,@uppy/core,@uppy/dashboard,@uppy/golden-retriever,@uppy/status-bar,@uppy/utils: Migrate all lodash' per-method-packages usage to lodash. (LinusMain / #4274) - @uppy/core: Don't set late (throttled) progress event on a file that is 100% complete (Artur Paikin / #4507) - @uppy/companion: revert randomness from file names (Mikael Finstad / #4509) - @uppy/companion: Custom provider fixes (Mikael Finstad / #4498) - @uppy/transloadit: ensure `fields` is not nullish when there no uploaded files (Antoine du Hamel / #4487) - @uppy/aws-s3-multipart,@uppy/aws-s3,@uppy/tus,@uppy/utils,@uppy/xhr-upload: When file is removed (or all are canceled), controller.abort queued requests (Artur Paikin / #4504) - @uppy/provider-views: Fix range selection not resetting and computing correctly (Terence C / #4415) - meta: disallow use of `.only` in tests (Antoine du Hamel / #4494) - @uppy/companion: fix 500 when file name contains non-ASCII chars (Antoine du Hamel / #4493) - @uppy/locales: update `fr_FR.js` (Samuel De Backer / #4499) - @uppy/aws-s3-multipart,@uppy/tus,@uppy/xhr-upload: Don't close socket while upload is still in progress (Artur Paikin / #4479) - meta: bump `luxon` from 1.28.0 to 1.28.1 (dependabot[bot] / #4497) - @uppy/utils: rename `EventTracker` -> `EventManager` (Stephen Wooten / #4481) - meta: bump cookiejar from 2.1.3 to 2.1.4 (dependabot[bot] / #4496) - meta: make `pre-commit` use `corepack yarn` instead of `npm run` (Antoine du Hamel / #4495) - meta: bump ua-parser-js from 0.7.31 to 0.7.35 (dependabot[bot] / #4474) - meta: bump @sideway/formula from 3.0.0 to 3.0.1 (dependabot[bot] / #4473) - meta: bump http-cache-semantics from 4.1.0 to 4.1.1 (dependabot[bot] / #4472) - @uppy/companion: Use filename from content-disposition instead of relying on url, with fallback (Artur Paikin / #4489) - meta: bump `babel`, `esbuild`, and `vite` (dependabot[bot] / #4485) - @uppy/dashboard: include the old state when setting new (Artur Paikin / #4490) - @uppy/companion: fix companion implicitpath (Mikael Finstad / #4484) - @uppy/companion: fix undefined protocol and example page (Mikael Finstad / #4483) - meta: upgrade Cypress 12.9.0 -> 12.14.0 (Antoine du Hamel / #4491) - @uppy/core: remove `state` getter from types (Antoine du Hamel / #4477) - examples/php-xhr: Added filename sanitation and file size check before saving (neuronet77 / #4432) - examples/php-xhr: update PHP dependencies (dependabot[bot]) - @uppy/xhr-upload: add support for arrays in metadata (Vasiliy Matyushin / #4431) - @uppy/status-bar: Filtered ETA (stduhpf / #4458) - @uppy/aws-s3-multipart: fix `getUploadParameters` option (Antoine du Hamel / #4465)
Murderlon
added a commit
that referenced
this pull request
Jun 20, 2023
* main: (61 commits) Release: uppy@3.10.0 (#4511) @uppy/aws-s3-multipart: fix the chunk size calculation (#4508) @uppy/aws-s3: add `shouldUseMultipart` option (#4299) @uppy/companion: switch from aws-sdk v2 to @aws-sdk/* (v3) (#4285) Migrate all lodash' per-method-packages usage to lodash. (#4274) @uppy/core: Don't set late (throttled) progress event on a file that is 100% complete (#4507) @uppy/companion: revert randomness from file names (#4509) Custom provider fixes (#4498) @uppy/transloadit: ensure `fields` is not nullish when there no uploaded files (#4487) When file is removed (or all are canceled), controller.abort queued requests (#4504) @uppy/provider-views: Fix range selection not resetting and computing correctly (#4415) meta: disallow use of `.only` in tests (#4494) @uppy/companion: fix 500 when file name contains non-ASCII chars (#4493) @uppy/locales: update `fr_FR.js` (#4499) Don't close socket while upload is still in progress (#4479) meta: bump `luxon` from 1.28.0 to 1.28.1 (#4497) @uppy/utils: rename `EventTracker` -> `EventManager` (#4481) meta: bump cookiejar from 2.1.3 to 2.1.4 (#4496) meta: make `pre-commit` use `corepack yarn` instead of `npm run` (#4495) meta: bump ua-parser-js from 0.7.31 to 0.7.35 (#4474) ...
Murderlon
added a commit
that referenced
this pull request
Jun 20, 2023
* main: Release: uppy@3.10.0 (#4511) @uppy/aws-s3-multipart: fix the chunk size calculation (#4508) @uppy/aws-s3: add `shouldUseMultipart` option (#4299) @uppy/companion: switch from aws-sdk v2 to @aws-sdk/* (v3) (#4285) Migrate all lodash' per-method-packages usage to lodash. (#4274) @uppy/core: Don't set late (throttled) progress event on a file that is 100% complete (#4507) @uppy/companion: revert randomness from file names (#4509) Custom provider fixes (#4498) @uppy/transloadit: ensure `fields` is not nullish when there no uploaded files (#4487) When file is removed (or all are canceled), controller.abort queued requests (#4504) @uppy/provider-views: Fix range selection not resetting and computing correctly (#4415) meta: disallow use of `.only` in tests (#4494)
2 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.
The
shouldUseMultipartoption is a way for the user to opt-in to mix multipart/non-multipart upload. When the option is set to something that's notfalse, the@uppy/aws-s3-multipartwill be used instead, a future version of Uppy will merge both plugins into one.Blocked on #4205.