@tus/s3-store: add maxMultipartParts option
#712
Conversation
🦋 Changeset detectedLatest commit: f3ee9d2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/tricky-emus-fail.md(1 hunks)packages/s3-store/src/index.ts(3 hunks)packages/s3-store/test/index.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/s3-store/test/index.ts
[error] 281-281: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.
(lint/suspicious/noFocusedTests)
[error] 281-286: This function expression can be turned into an arrow function.
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
(lint/complexity/useArrowFunction)
[error] 288-288: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.
(lint/suspicious/noFocusedTests)
[error] 288-295: This function expression can be turned into an arrow function.
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
(lint/complexity/useArrowFunction)
🪛 GitHub Actions: CI
packages/s3-store/src/index.ts
[error] 110-110: Formatter would have printed the following content: const { maxMultipartParts, partSize, minPartSize, s3ClientConfig } = options
🔇 Additional comments (3)
packages/s3-store/src/index.ts (2)
104-104: LGTM!The property declaration is correct. Removing the
as constassertion is appropriate since the value can be modified through the constructor.
522-528: LGTM!The logic for calculating optimal part size based on
maxMultipartPartsis correct and well-documented. It ensures that:
- If the upload size is smaller than the preferred part size, it uses a single part.
- If the upload fits within
maxMultipartPartsusing the preferred part size, it uses the preferred size.- Otherwise, it calculates the minimum part size needed to fit within
maxMultipartParts..changeset/tricky-emus-fail.md (1)
1-6: LGTM!The changeset correctly indicates a minor version bump for adding a new feature. The description clearly explains the purpose of the
maxMultipartPartsoption.
|
It is noisy, but parts of it make sense. Just when I figured I need to remove Are test set to fail if it encounters focused tests? (using |
The linting step (Biome) should fail on this yes |
* 'main' of https://github.com/tus/tus-node-server: @tus/s3-store: add `maxMultipartParts` option (#712) [ci] release (#701) @tus/s3-store: add `minPartSize` option (#703)
This PR closes #711.
I added additional option
maxMultipartPartswhich allows user to define non-standard S3 server limitations.Also some tests for completeness.
Summary by CodeRabbit
New Features
Tests