fix: reconsider behavior when providing a value lower than 500 for bu…#438
fix: reconsider behavior when providing a value lower than 500 for bu…#438harsh082ip wants to merge 3 commits intocivo:masterfrom
Conversation
|
@fulviodenza are you free to give it a look? |
|
Hi @harsh082ip, thanks for your contribution! While the original issue has already been marked as closed and some parts of it were resolved, I think we should discuss whether we want to let the @alessandroargentieri @fulviodenza @uzaxirr thoughts? |
|
Hii @giornetta , Thanks for the context! I agree, auto-selecting a higher size with --yes might lead to unexpected charges. Happy to align on the best approach. |
cmd/objectstore/objectstore.go
Outdated
|
|
||
| //Flags for create cmd | ||
| objectStoreCreateCmd.Flags().Int64VarP(&bucketSize, "size", "s", 500, "Size of the Object store") | ||
| objectStoreCreateCmd.Flags().Int64VarP(&bucketSize, "size", "s", 500, "Size of the Object store in GB (minimum: 500)") |
There was a problem hiding this comment.
let's use a variable for this 500 value,
| os.Exit(1) | ||
| } | ||
| if !accept { | ||
| os.Exit(0) |
There was a problem hiding this comment.
this indicates success, the operation, when not accepted is not a success, we need os.Exit(1) for this
| utility.Error("Unable to parse the input: %s", err) | ||
| os.Exit(1) | ||
| if !yesFlag { | ||
| utility.YellowConfirm("The size to create an object store must be a multiple of 500. Would you like to create an %s of %d GB instead? (y/n) ? ", utility.Green("object store"), bucketSize+(500-bucketSize%500)) |
There was a problem hiding this comment.
we should specify that 500 and 1000GB are the only allowed sizes
There was a problem hiding this comment.
bucketSize+(500-bucketSize%500)) could be a small function
|
Hi @fulviodenza, apologies for the delay — I've pushed the required changes. Let me know if anything else is needed. |
|
|
||
| //Flags for create cmd | ||
| objectStoreCreateCmd.Flags().Int64VarP(&bucketSize, "size", "s", 500, "Size of the Object store (Minimum size is 500GB)") | ||
| objectStoreCreateCmd.Flags().Int64VarP(&bucketSize, "size", "s", minBucketSizeGB, fmt.Sprintf("Size of the Object store (Allowed sizes: %dGB or 1000GB)", minBucketSizeGB)) |
There was a problem hiding this comment.
why the 500 is set as constant while the 1000 isn't?
| Example: "civo objectstore create OBJECTSTORE_NAME --size SIZE", | ||
| Short: "Create a new Object Store", | ||
| Long: "Bucket size should be in Gigabytes (GB) and must be a multiple of 500, starting from 500.\n", | ||
| Long: "Bucket size should be in Gigabytes (GB) and must be either 500 or 1000.\n", |
There was a problem hiding this comment.
couldn't we use the constants of before, declared in a common package?
4 participants
As mentioned in issue #394, the following changes have been made:
The command will now fail immediately for values less than 500 GB with an appropriate error message.
Introduced a --yes flag to automatically adjust the bucket size to the nearest multiple of 500 without prompting the user.
For example:
Warning: The size to create an object store must be a multiple of 500. Would you like to create an object store of 1000 GB instead? (y/n)?
Using the --yes flag, users can bypass this prompt.
Improved the help message to make the minimum and default size requirements explicit:
-s, --size int Size of the Object store in GB (minimum: 500) (default 500)
Screenshot:
Please review and let me know if any changes are needed.
@haardikdharma10 @uzaxirr @RealHarshThakur @alejandrojnm