Skip to content

Commit f5ab15d

Browse files
committed
fix code review comments
1 parent ddf5906 commit f5ab15d

12 files changed

Lines changed: 124 additions & 64 deletions

File tree

internal/cmd/postgresflex/backup/list/list.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,8 @@ func NewCmd(params *types.CmdParams) *cobra.Command {
7676
if err != nil {
7777
return fmt.Errorf("get backups for PostgreSQL Flex instance %q: %w", instanceLabel, err)
7878
}
79-
backups := resp.Backups
8079

81-
// Truncate output
82-
if model.Limit != nil && len(backups) > int(*model.Limit) {
83-
backups = backups[:*model.Limit]
84-
}
85-
86-
return outputResult(params.Printer, model.OutputFormat, instanceLabel, backups)
80+
return outputResult(params.Printer, model.OutputFormat, instanceLabel, resp.Backups)
8781
},
8882
}
8983

@@ -122,6 +116,14 @@ func parseInput(p *print.Printer, cmd *cobra.Command, _ []string) (*inputModel,
122116

123117
func buildRequest(ctx context.Context, model *inputModel, apiClient *postgresflex.APIClient) postgresflex.ApiListBackupsRequest {
124118
req := apiClient.DefaultAPI.ListBackups(ctx, model.ProjectId, model.Region, *model.InstanceId)
119+
120+
if model.Limit != nil {
121+
req = req.Size(*model.Limit)
122+
} else {
123+
// default page size is only 10
124+
req = req.Size(100)
125+
}
126+
125127
return req
126128
}
127129

@@ -133,7 +135,7 @@ func outputResult(p *print.Printer, outputFormat, instanceLabel string, backups
133135
}
134136

135137
table := tables.NewTable()
136-
table.SetHeader("ID", "CREATED AT", "RETAINED UNTIL", "BACKUP SIZE")
138+
table.SetHeader("ID", "COMPLETED AT", "RETAINED UNTIL", "BACKUP SIZE")
137139

138140
for _, backup := range backups {
139141
backupCompletionTime, err := time.Parse(time.RFC3339, backup.CompletionTime)

internal/cmd/postgresflex/backup/list/list_test.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func fixtureInputModel(mods ...func(model *inputModel)) *inputModel {
5656
}
5757

5858
func fixtureRequest(mods ...func(request *postgresflex.ApiListBackupsRequest)) postgresflex.ApiListBackupsRequest {
59-
request := testClient.DefaultAPI.ListBackups(testCtx, testProjectId, testRegion, testInstanceId)
59+
request := testClient.DefaultAPI.ListBackups(testCtx, testProjectId, testRegion, testInstanceId).Size(100)
6060
for _, mod := range mods {
6161
mod(&request)
6262
}
@@ -154,10 +154,21 @@ func TestBuildRequest(t *testing.T) {
154154
expectedRequest postgresflex.ApiListBackupsRequest
155155
}{
156156
{
157-
description: "base",
158-
model: fixtureInputModel(),
157+
description: "base",
158+
model: fixtureInputModel(func(model *inputModel) {
159+
model.Limit = nil
160+
}),
159161
expectedRequest: fixtureRequest(),
160162
},
163+
{
164+
description: "limit flag is set",
165+
model: fixtureInputModel(func(model *inputModel) {
166+
model.Limit = new(int64(12))
167+
}),
168+
expectedRequest: fixtureRequest(func(request *postgresflex.ApiListBackupsRequest) {
169+
*request = request.Size(12)
170+
}),
171+
},
161172
}
162173

163174
for _, tt := range tests {

internal/cmd/postgresflex/flavor/list/list.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,20 @@ import (
1212
"github.com/stackitcloud/stackit-cli/internal/pkg/args"
1313
cliErr "github.com/stackitcloud/stackit-cli/internal/pkg/errors"
1414
"github.com/stackitcloud/stackit-cli/internal/pkg/examples"
15+
"github.com/stackitcloud/stackit-cli/internal/pkg/flags"
1516
"github.com/stackitcloud/stackit-cli/internal/pkg/globalflags"
1617
"github.com/stackitcloud/stackit-cli/internal/pkg/print"
1718
"github.com/stackitcloud/stackit-cli/internal/pkg/services/postgresflex/client"
1819
"github.com/stackitcloud/stackit-cli/internal/pkg/tables"
1920
)
2021

22+
const (
23+
limitFlag = "limit"
24+
)
25+
2126
type inputModel struct {
2227
*globalflags.GlobalFlagModel
28+
Limit *int64
2329
}
2430

2531
func NewCmd(params *types.CmdParams) *cobra.Command {
@@ -55,25 +61,49 @@ func NewCmd(params *types.CmdParams) *cobra.Command {
5561
return outputResult(params.Printer, model.OutputFormat, flavors.Flavors)
5662
},
5763
}
64+
65+
configureFlags(cmd)
5866
return cmd
5967
}
6068

69+
func configureFlags(cmd *cobra.Command) {
70+
cmd.Flags().Int64(limitFlag, 0, "Maximum number of entries to list")
71+
}
72+
6173
func parseInput(p *print.Printer, cmd *cobra.Command, _ []string) (*inputModel, error) {
6274
globalFlags := globalflags.Parse(p, cmd)
6375
if globalFlags.ProjectId == "" {
6476
return nil, &cliErr.ProjectIdError{}
6577
}
6678

79+
limit := flags.FlagToInt64Pointer(p, cmd, limitFlag)
80+
if limit != nil && *limit < 1 {
81+
return nil, &cliErr.FlagValidationError{
82+
Flag: limitFlag,
83+
Details: "must be greater than 0",
84+
}
85+
}
86+
6787
model := inputModel{
6888
GlobalFlagModel: globalFlags,
89+
Limit: limit,
6990
}
7091

7192
p.DebugInputModel(model)
7293
return &model, nil
7394
}
7495

7596
func buildRequest(ctx context.Context, model *inputModel, apiClient postgresflex.DefaultAPI) postgresflex.ApiListFlavorsRequest {
76-
return apiClient.ListFlavors(ctx, model.ProjectId, model.Region).Size(100)
97+
req := apiClient.ListFlavors(ctx, model.ProjectId, model.Region)
98+
99+
if model.Limit != nil {
100+
req = req.Size(*model.Limit)
101+
} else {
102+
// the default page size is only 10
103+
req = req.Size(100)
104+
}
105+
106+
return req
77107
}
78108

79109
func outputResult(p *print.Printer, outputFormat string, flavors []postgresflex.ListFlavors) error {

internal/cmd/postgresflex/flavor/list/list_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,15 @@ func TestBuildRequest(t *testing.T) {
105105
model: fixtureInputModel(),
106106
expectedRequest: fixtureRequest(),
107107
},
108+
{
109+
description: "limit flag set",
110+
model: fixtureInputModel(func(model *inputModel) {
111+
model.Limit = new(int64(12))
112+
}),
113+
expectedRequest: fixtureRequest(func(request *postgresflex.ApiListFlavorsRequest) {
114+
*request = request.Size(12)
115+
}),
116+
},
108117
}
109118

110119
for _, tt := range tests {

internal/cmd/postgresflex/instance/create/create.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ func configureFlags(cmd *cobra.Command) {
216216
cmd.MarkFlagsRequiredTogether(encryptionKekKeyIdFlag, encryptionKekKeyringIdFlag, encryptionKekKeyVersionFlag, encryptionServiceAccountFlag)
217217

218218
// remove after 2027-01-31
219-
err = cmd.Flags().MarkDeprecated("type", fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag))
219+
err = cmd.Flags().MarkDeprecated(typeFlag.Name(), fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag))
220220
cobra.CheckErr(err)
221221
err = cmd.Flags().MarkDeprecated(cpuFlag, fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag))
222222
cobra.CheckErr(err)

internal/cmd/postgresflex/instance/list/list.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,19 +68,13 @@ func NewCmd(params *types.CmdParams) *cobra.Command {
6868
return fmt.Errorf("get PostgreSQL Flex instances: %w", err)
6969
}
7070

71-
instances := resp.Instances
72-
73-
// Truncate output
74-
if model.Limit != nil && len(instances) > int(*model.Limit) {
75-
instances = instances[:*model.Limit]
76-
}
77-
7871
projectLabel, err := projectname.GetProjectName(ctx, params.Printer, params.CliVersion, cmd)
7972
if err != nil {
8073
params.Printer.Debug(print.ErrorLevel, "get project name: %v", err)
8174
projectLabel = model.ProjectId
8275
}
83-
return outputResult(params.Printer, model.OutputFormat, projectLabel, instances)
76+
77+
return outputResult(params.Printer, model.OutputFormat, projectLabel, resp.Instances)
8478
},
8579
}
8680

@@ -117,6 +111,14 @@ func parseInput(p *print.Printer, cmd *cobra.Command, _ []string) (*inputModel,
117111

118112
func buildRequest(ctx context.Context, model *inputModel, apiClient *postgresflex.APIClient) postgresflex.ApiListInstancesRequest {
119113
req := apiClient.DefaultAPI.ListInstances(ctx, model.ProjectId, model.Region)
114+
115+
if model.Limit != nil {
116+
req = req.Size(*model.Limit)
117+
} else {
118+
// default page size is only 10
119+
req = req.Size(100)
120+
}
121+
120122
return req
121123
}
122124

internal/cmd/postgresflex/instance/list/list_test.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func fixtureInputModel(mods ...func(model *inputModel)) *inputModel {
5050
}
5151

5252
func fixtureRequest(mods ...func(request *postgresflex.ApiListInstancesRequest)) postgresflex.ApiListInstancesRequest {
53-
request := testClient.DefaultAPI.ListInstances(testCtx, testProjectId, testRegion)
53+
request := testClient.DefaultAPI.ListInstances(testCtx, testProjectId, testRegion).Size(100)
5454
for _, mod := range mods {
5555
mod(&request)
5656
}
@@ -127,10 +127,21 @@ func TestBuildRequest(t *testing.T) {
127127
expectedRequest postgresflex.ApiListInstancesRequest
128128
}{
129129
{
130-
description: "base",
131-
model: fixtureInputModel(),
130+
description: "base",
131+
model: fixtureInputModel(func(model *inputModel) {
132+
model.Limit = nil
133+
}),
132134
expectedRequest: fixtureRequest(),
133135
},
136+
{
137+
description: "limit flag is set",
138+
model: fixtureInputModel(func(model *inputModel) {
139+
model.Limit = new(int64(12))
140+
}),
141+
expectedRequest: fixtureRequest(func(request *postgresflex.ApiListInstancesRequest) {
142+
*request = request.Size(12)
143+
}),
144+
},
134145
}
135146

136147
for _, tt := range tests {

internal/cmd/postgresflex/instance/update/update.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,20 +115,26 @@ func NewCmd(params *types.CmdParams) *cobra.Command {
115115
return fmt.Errorf("update PostgreSQL Flex instance: %w", err)
116116
}
117117

118-
var waitResp *postgresflex.GetInstanceResponse
118+
// update endpoint doesn't return the updated instance, so we have to call the GET endpoint to fetch it
119+
var getResp *postgresflex.GetInstanceResponse
119120

120121
// Wait for async operation, if async mode not enabled
121122
if !model.Async {
122123
err := spinner.Run(params.Printer, "Updating instance", func() error {
123-
waitResp, err = wait.PartialUpdateInstanceWaitHandler(ctx, apiClient.DefaultAPI, model.ProjectId, model.Region, model.InstanceId).WaitWithContext(ctx)
124+
getResp, err = wait.PartialUpdateInstanceWaitHandler(ctx, apiClient.DefaultAPI, model.ProjectId, model.Region, model.InstanceId).WaitWithContext(ctx)
124125
return err
125126
})
126127
if err != nil {
127128
return fmt.Errorf("wait for PostgreSQL Flex instance update: %w", err)
128129
}
130+
} else {
131+
getResp, err = apiClient.DefaultAPI.GetInstance(ctx, model.ProjectId, model.Region, model.InstanceId).Execute()
132+
if err != nil {
133+
return fmt.Errorf("fetching PostgreSQL Flex instance after async update: %w", err)
134+
}
129135
}
130136

131-
return outputResult(params.Printer, model.OutputFormat, model.Async, instanceLabel, waitResp)
137+
return outputResult(params.Printer, model.OutputFormat, model.Async, instanceLabel, getResp)
132138
},
133139
}
134140
configureFlags(cmd)
@@ -153,7 +159,7 @@ func configureFlags(cmd *cobra.Command) {
153159
cobra.CheckErr(err)
154160

155161
// remove after 2027-01-31
156-
err = cmd.Flags().MarkDeprecated("type", fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag))
162+
err = cmd.Flags().MarkDeprecated(typeFlag.Name(), fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag))
157163
cobra.CheckErr(err)
158164
err = cmd.Flags().MarkDeprecated(cpuFlag, fmt.Sprintf("Will be removed after 2027-01-31. Use the --%s flag instead.", flavorIdFlag))
159165
cobra.CheckErr(err)

internal/cmd/postgresflex/user/list/list.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,14 @@ func parseInput(p *print.Printer, cmd *cobra.Command, _ []string) (*inputModel,
122122

123123
func buildRequest(ctx context.Context, model *inputModel, apiClient *postgresflex.APIClient) postgresflex.ApiListUsersRequest {
124124
req := apiClient.DefaultAPI.ListUsers(ctx, model.ProjectId, model.Region, model.InstanceId)
125+
126+
if model.Limit != nil {
127+
req = req.Size(*model.Limit)
128+
} else {
129+
// default page size is only 10
130+
req = req.Size(100)
131+
}
132+
125133
return req
126134
}
127135

internal/cmd/postgresflex/user/list/list_test.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func fixtureInputModel(mods ...func(model *inputModel)) *inputModel {
5353
}
5454

5555
func fixtureRequest(mods ...func(request *postgresflex.ApiListUsersRequest)) postgresflex.ApiListUsersRequest {
56-
request := testClient.DefaultAPI.ListUsers(testCtx, testProjectId, testRegion, testInstanceId)
56+
request := testClient.DefaultAPI.ListUsers(testCtx, testProjectId, testRegion, testInstanceId).Size(100)
5757
for _, mod := range mods {
5858
mod(&request)
5959
}
@@ -144,10 +144,21 @@ func TestBuildRequest(t *testing.T) {
144144
expectedRequest postgresflex.ApiListUsersRequest
145145
}{
146146
{
147-
description: "base",
148-
model: fixtureInputModel(),
147+
description: "base",
148+
model: fixtureInputModel(func(model *inputModel) {
149+
model.Limit = nil
150+
}),
149151
expectedRequest: fixtureRequest(),
150152
},
153+
{
154+
description: "limit flag is set",
155+
model: fixtureInputModel(func(model *inputModel) {
156+
model.Limit = new(int64(12))
157+
}),
158+
expectedRequest: fixtureRequest(func(request *postgresflex.ApiListUsersRequest) {
159+
*request = request.Size(12)
160+
}),
161+
},
151162
}
152163

153164
for _, tt := range tests {

0 commit comments

Comments
 (0)