feat(postgresflex): migrate to v3 API - #1505
Open
rubenhoenle wants to merge 3 commits into
Open
Conversation
rubenhoenle
marked this pull request as ready for review
July 29, 2026 13:31
relates to STACKITCLI-415
rubenhoenle
force-pushed
the
feat/postgresflex-v3-api
branch
from
July 29, 2026 13:31
477d84b to
1a8fed5
Compare
Merging this branch changes the coverage (11 decrease, 7 increase)
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
8 tasks
marceljk
reviewed
Jul 31, 2026
Comment on lines
+344
to
345
| Acl: model.ACL, | ||
| }, |
Contributor
There was a problem hiding this comment.
There is also a new additional property accessScope. AFAIK this is needed for the private endpoints feature
marceljk
reviewed
Jul 31, 2026
Comment on lines
+93
to
+96
| backupId, err := strconv.ParseInt(backupIdStr, 10, 64) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("invalid backup id format, must be an integer: %w", err) | ||
| } |
Contributor
There was a problem hiding this comment.
nitpick: you can also pass a validate function in the call of args.SingleArg(backupIdArg, <function>) and check there if the input is valid.
But it's anyways required to parse here the string to int, so the validate function wouldn't have a big benefit.
Contributor
There was a problem hiding this comment.
Feel free to just close the comment, if you don't want to add the validate function
| table.SetHeader("ID", "CREATED AT", "EXPIRES AT", "BACKUP SIZE") | ||
| for i := range backups { | ||
| backup := backups[i] | ||
| table.SetHeader("ID", "CREATED AT", "RETAINED UNTIL", "BACKUP SIZE") |
Contributor
There was a problem hiding this comment.
Suggested change
| table.SetHeader("ID", "CREATED AT", "RETAINED UNTIL", "BACKUP SIZE") | |
| table.SetHeader("ID", "COMPLETED AT", "RETAINED UNTIL", "BACKUP SIZE") |
| cmd.Flags().String(recoveryTimestampFlag, "", "Recovery timestamp for the instance, in a date-time with the layout format YYYY-MM-DDTHH:mm:ss±HH:mm, e.g. 2006-01-02T15:04:05-07:00") | ||
| cmd.Flags().String(storageClassFlag, "", "Storage class. If not specified, storage class from the existing instance will be used.") | ||
| cmd.Flags().Int64(storageSizeFlag, 0, "Storage size (in GB). If not specified, storage size from the existing instance will be used.") | ||
| cmd.Flags().String(storageClassFlag, "", "Storage class. If not specified, storage class from the existing instance will be used. This flag will be required after 2027-01-31.") |
Contributor
There was a problem hiding this comment.
Didn't test the endpoint here, but based on the API Docs only storageSize is required, but not storageClass. Do we need to make this required in the future or what will happen if isn't just not set?
| cmd.MarkFlagsRequiredTogether(encryptionKekKeyIdFlag, encryptionKekKeyringIdFlag, encryptionKekKeyVersionFlag, encryptionServiceAccountFlag) | ||
|
|
||
| // remove after 2027-01-31 | ||
| err = cmd.Flags().MarkDeprecated("type", fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag)) |
Contributor
There was a problem hiding this comment.
Suggested change
| err = cmd.Flags().MarkDeprecated("type", fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag)) | |
| err = cmd.Flags().MarkDeprecated(typeFlag.Name(), fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag)) |
| cobra.CheckErr(err) | ||
|
|
||
| // remove after 2027-01-31 | ||
| err = cmd.Flags().MarkDeprecated("type", fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag)) |
Contributor
There was a problem hiding this comment.
Suggested change
| err = cmd.Flags().MarkDeprecated("type", fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag)) | |
| err = cmd.Flags().MarkDeprecated(typeFlag.Name(), fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag)) |
| func ValidateStorage(storageClass *string, storageSize *int64, storages *postgresflex.ListStoragesResponse, flavorId string) error { | ||
| if storages == nil { | ||
| return fmt.Errorf("nil storages") | ||
| /*func ValidateStorage(storageClass *string, storageSize *int64, flavors []postgresflex.ListFlavors, flavorId string) error { |
marceljk
reviewed
Jul 31, 2026
| return &f.Id, nil | ||
| } | ||
| availableFlavors = fmt.Sprintf("%s\n- %d CPU, %d GB RAM", availableFlavors, *f.Cpu, *f.Cpu) | ||
| availableFlavors = fmt.Sprintf("%s\n- %d CPU, %d GB RAM", availableFlavors, f.Cpu, f.Cpu) |
Contributor
There was a problem hiding this comment.
Suggested change
| availableFlavors = fmt.Sprintf("%s\n- %d CPU, %d GB RAM", availableFlavors, f.Cpu, f.Cpu) | |
| availableFlavors = fmt.Sprintf("%s\n- %d CPU, %d GB RAM", availableFlavors, f.Cpu, f.Memory) |
Contributor
There was a problem hiding this comment.
availableFlavors isn't used at all, so this can be removed
Comment on lines
+118
to
121
| var waitResp *postgresflex.GetInstanceResponse | ||
|
|
||
| // Wait for async operation, if async mode not enabled | ||
| if !model.Async { |
Contributor
There was a problem hiding this comment.
When the update command is executed with the flag --async, the waitResp remains nil and results in an error output
marceljk
reviewed
Jul 31, 2026
| backups := resp.Backups | ||
|
|
||
| // Truncate output | ||
| if model.Limit != nil && len(backups) > int(*model.Limit) { |
Contributor
There was a problem hiding this comment.
The used endpoint supports setting a page size. I think at this point we can just set the page size with the limit. Then we don't need to truncate the output manually and fetch the right amount of results
Contributor
There was a problem hiding this comment.
same applies to the other list endpoints
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
2 participants
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.
Description
relates to STACKITCLI-415
Checklist
make fmtmake generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)