Repository navigation
Reject negative volume transfer flags and fix default help text - #266
Open
vincent0426 wants to merge 1 commit into
Open
vincent0426 wants to merge 1 commit into
vincent0426 wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Regression tests cover only --file-jobs on push, leaving negative-value rejection for --chunk-operations and --max-in-flight-mib and validation wiring into pull/cat untested.
1 open issue
What changed in this PR
This PR makes volume transfer commands reject negative concurrency limits with a usage error, while preserving zero as the default.
Changes:
- Validate transfer limits before push, pull, and cat begin work.
- Correct
--file-jobshelp to describe the default of 256 or the configured--chunk-operationsvalue. - Add a regression test for negative
--file-jobs, checking the error message and exit code 2.
| File | Description |
|---|---|
internal/cmd/command.volume_transfer.go |
Validate negative limits and propagate usage errors. |
internal/cmd/command.volume_test.go |
Test rejection of negative --file-jobs on push. |
cmd/command.volume.go |
Correct the documented file concurrency default. |
Review effort: Balanced
Comment on lines
+1956
to
+1961
| func Test_Volume_Push_RejectsNegativeTransferFlag(t *testing.T) { | ||
| h := NewCommandHarness(t) | ||
| err := h.Execute("volume", "push", t.TempDir(), "bdn:ns/vol", "--file-jobs", "-1") | ||
| h.Require.ErrorContains(err, "--file-jobs must not be negative") | ||
| h.Require.Equal(2, h.ExitCode) | ||
| } |
This file contains hidden or 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
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.
🚀 What
--file-jobs,--chunk-operations,--max-in-flight-mibas default -> now fail with a usage error.--file-jobshelp said defaults is 16. The real default is 256, or--chunk-operationswhen that is set.💻 How
volumeConcurrency rejects negative values before the command starts. Zero still means default.
https://github.com/basetenlabs/baseten-go/blob/12644756ba3b66421524e1fcb0dd21d1136f254f/internal/volume/limiter.go#L232-L252
🔬 Testing
Added a test: volume push --file-jobs -1 exits 2 with the new message.