Conversation
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
fd3fc6c to
0679364
Compare
| if err != nil { | ||
| return req, err | ||
| for _, flavor := range flavors.Flavors { | ||
| if *flavor.Cpu == *model.CPU && *flavor.Memory == *model.RAM { |
There was a problem hiding this comment.
Potential nil pointer dereference when API does not specify CPU or memory
| return req, fmt.Errorf("get MongoDB Flex instance: %w", err) | ||
| var currentFlavor *mongodbflex.InstanceFlavor | ||
| for _, f := range flavors.Flavors { | ||
| if *f.Id == *currentInstance.Item.Flavor.Id { |
There was a problem hiding this comment.
Here also potential nil pointer dereference. Probably won't happen in practice, but since the fields are optional in the API i think we should treat them as potentially nullable
| currentInstance, err := apiClient.GetInstance(ctx, model.ProjectId, model.InstanceId, model.Region).Execute() | ||
| if err != nil { | ||
| return req, fmt.Errorf("get MongoDB Flex instance: %w", err) | ||
| } |
There was a problem hiding this comment.
We can move the GetInstance call inside the if blocks below, since we only need to call it when the user only specifies either CPU or RAM
| versions, err := buildRequest(ctx, model, apiClient.DefaultAPI).Execute() | ||
| if err != nil { | ||
| return fmt.Errorf("get MongoDB Flex versions: %w", err) | ||
| } | ||
|
|
||
| return outputResult(params.Printer, model.OutputFormat, versions.Versions) |
There was a problem hiding this comment.
--limit support missing? Or is it not required here?
There was a problem hiding this comment.
I think --limit is not needed in here. we only have 2 versions. In other db services, version list also does not have limit
relates to STACKITCLI-441
relates to STACKITCLI-441
Co-authored-by: Alexander Dahmen <alexander.dahmen@inovex.de>
relates to STACKITCLI-441
Co-authored-by: Jonas Schlecht <73650029+SerseusWasTaken@users.noreply.github.com>
1fbf61d to
dcbe4c5
Compare
relates to STACKITCLI-441
Merging this branch changes the coverage (4 decrease, 11 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
|
Description
relates to STACKITCLI-441
Checklist
make fmtmake generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)