@uppy/aws-s3-multipart: refactor rate limiting approach #4187
+494
−503
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
mifi
reviewed
Nov 4, 2022
Murderlon
reviewed
Nov 7, 2022
Murderlon
reviewed
Nov 7, 2022
* remove dead code and simplify * Apply suggestions from code review Co-authored-by: Mikael Finstad <finstaden@gmail.com> Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95
force-pushed
the
sw-multipart-queue
branch
from
f0eef90 to
f807ffa
Compare
Murderlon
reviewed
Nov 8, 2022
Murderlon
reviewed
Nov 8, 2022
Murderlon
approved these changes
Nov 8, 2022
Murderlon
reviewed
Nov 8, 2022
Murderlon
reviewed
Nov 9, 2022
|
|
||
| async #handleError (err) { | ||
| const requests = this.#requests | ||
| const status = err?.source?.status |
There was a problem hiding this comment.
I see we set error.source when an error occurred for the PUT request, but does the signPart also return source now?
There was a problem hiding this comment.
No, it's a request to Companion, it doesn't have anything of the sort (and it should not, we should have a retry logic within companion-client)
There was a problem hiding this comment.
So what if signPart fails? I'd say we want to have retry logic not all over the place and in one place. So it would retry here even though it may offload the request logic to companion client?
Merged
github-actions bot
added a commit
that referenced
this pull request
Nov 10, 2022
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/angular | 0.5.0 | @uppy/image-editor | 2.1.0 | | @uppy/aws-s3-multipart | 3.1.0 | @uppy/locales | 3.0.4 | | @uppy/companion | 4.1.0 | @uppy/tus | 3.0.5 | | @uppy/companion-client | 3.1.0 | @uppy/utils | 5.1.0 | | @uppy/dashboard | 3.2.0 | uppy | 3.3.0 | - @uppy/companion: change default S3 expiry from 300 to 800 seconds (Merlijn Vos / #4206) - @uppy/dashboard: Single file mode (Artur Paikin / #4188) - @uppy/locales: Fix UZ locale (Merlijn Vos / #4178) - @uppy/utils: update typings for `RateLimitedQueue` (Antoine du Hamel / #4204) - @uppy/aws-s3-multipart: empty the queue when pausing (Antoine du Hamel / #4203) - @uppy/image-editor: add checkered background (Livia Medeiros / #4194) - @uppy/aws-s3-multipart: refactor rate limiting approach (Antoine du Hamel / #4187) - @uppy/companion: send expiry time along side S3 signed requests (Antoine du Hamel / #4202) - @uppy/companion-client: add support for `AbortSignal` (Antoine du Hamel / #4201) - @uppy/companion-client: prevent preflight race condition (Mikael Finstad / #4182) - @uppy/aws-s3-multipart: change limit to 6 (Antoine du Hamel / #4199) - @uppy/utils: add `cause` support for `AbortError`s (Antoine du Hamel / #4198) - meta: Fix bad example for setFileState (Tim Whitney / #4191) - meta: Update code example for getFiles (Tim Whitney / #4189) - meta: Fix issue with outdated comment. (Tim Whitney / #4192) - @uppy/aws-s3-multipart: remove unused `timeout` option (Antoine du Hamel / #4186) - meta: Remove dollar sign from command for easier copy/pasting (Youssef Victor / #4180) - @uppy/aws-s3-multipart,@uppy/tus: fix `Timed out waiting for socket` (Antoine du Hamel / #4177) - meta: Add note about facebook approval (Mikael Finstad / #4172) - meta: add a manual deploy for website (Antoine du Hamel / #4171)
Murderlon
added a commit
that referenced
this pull request
Dec 1, 2022
* main: (110 commits) @uppy/aws-s3-multipart: handle slow connections better (#4213) @uppy/companion-client: treat `*` the same as missing header (#4221) @uppy/utils: fix types (#4212) @uppy/companion: send expire info for non-multipart uploads (#4214) docs: fix `allowedMetaFields` documentation (#4216) meta: add more bundlers for automated testing (#4100) @uppy/aws-s3-multipart: Fix typo in url check (#4211) meta: use current version of packages when testing bundlers (#4208) meta: do not use the set-output command in workflows (#4175) Release: uppy@3.3.0 (#4207) Companion: change default S3 expiry from 300 to 800 seconds (#4206) Dashboard: Single file mode (#4188) Fix UZ locale (#4178) @uppy/utils: update typings for `RateLimitedQueue` (#4204) @uppy/aws-s3-multipart: empty the queue when pausing (#4203) image-editor: add checkered background (#4194) @uppy/aws-s3-multipart: refactor rate limiting approach (#4187) @uppy/companion: send expiry time along side S3 signed requests (#4202) @uppy/companion-client: add support for `AbortSignal` (#4201) @uppy/companion-client: prevent preflight race condition (#4182) ...
HeavenFox
pushed a commit
to docsend/uppy
that referenced
this pull request
Jun 27, 2023
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/angular | 0.5.0 | @uppy/image-editor | 2.1.0 | | @uppy/aws-s3-multipart | 3.1.0 | @uppy/locales | 3.0.4 | | @uppy/companion | 4.1.0 | @uppy/tus | 3.0.5 | | @uppy/companion-client | 3.1.0 | @uppy/utils | 5.1.0 | | @uppy/dashboard | 3.2.0 | uppy | 3.3.0 | - @uppy/companion: change default S3 expiry from 300 to 800 seconds (Merlijn Vos / transloadit#4206) - @uppy/dashboard: Single file mode (Artur Paikin / transloadit#4188) - @uppy/locales: Fix UZ locale (Merlijn Vos / transloadit#4178) - @uppy/utils: update typings for `RateLimitedQueue` (Antoine du Hamel / transloadit#4204) - @uppy/aws-s3-multipart: empty the queue when pausing (Antoine du Hamel / transloadit#4203) - @uppy/image-editor: add checkered background (Livia Medeiros / transloadit#4194) - @uppy/aws-s3-multipart: refactor rate limiting approach (Antoine du Hamel / transloadit#4187) - @uppy/companion: send expiry time along side S3 signed requests (Antoine du Hamel / transloadit#4202) - @uppy/companion-client: add support for `AbortSignal` (Antoine du Hamel / transloadit#4201) - @uppy/companion-client: prevent preflight race condition (Mikael Finstad / transloadit#4182) - @uppy/aws-s3-multipart: change limit to 6 (Antoine du Hamel / transloadit#4199) - @uppy/utils: add `cause` support for `AbortError`s (Antoine du Hamel / transloadit#4198) - meta: Fix bad example for setFileState (Tim Whitney / transloadit#4191) - meta: Update code example for getFiles (Tim Whitney / transloadit#4189) - meta: Fix issue with outdated comment. (Tim Whitney / transloadit#4192) - @uppy/aws-s3-multipart: remove unused `timeout` option (Antoine du Hamel / transloadit#4186) - meta: Remove dollar sign from command for easier copy/pasting (Youssef Victor / transloadit#4180) - @uppy/aws-s3-multipart,@uppy/tus: fix `Timed out waiting for socket` (Antoine du Hamel / transloadit#4177) - meta: Add note about facebook approval (Mikael Finstad / transloadit#4172) - meta: add a manual deploy for website (Antoine du Hamel / transloadit#4171)
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.
Resurrection of #4179.