Conversation
kayrus
left a comment
There was a problem hiding this comment.
Looks like everything is already implemented. You just need to use some extra code to obtain the desired information.
| // used to avoid extra calls to Get/ListExtraSpecs. This is only available | ||
| // in microversion 2.61 and greater, and only if allowed by the compute | ||
| // service's policy. | ||
| ExtraSpecs map[string]string `json:"extra_specs"` |
There was a problem hiding this comment.
this is already implemented you need to use https://pkg.go.dev/github.com/gophercloud/gophercloud/openstack/compute/v2/flavors#ListExtraSpecs
There was a problem hiding this comment.
It is already implemented, but you can avoid the N extra round trips and just get the data directly in the /details call. So above and beyond the obvious performance improvement, and load reduction, it cleans up client code a lot, especially if you have to mess about with go routines to hide the extra calls.
There was a problem hiding this comment.
To prove my point of how bad it can be here's the naive version:
And when you add in a heap of code to parallelize everything:
Or, what this proposes, is neither of those things and just getting it in the original call.
fcf7b7f to
448ef7e
Compare
Add the "extra_specs" field in compute flavors, that can avoid another lookup. As documented it's for microversions 2.61 and greater. Documented here: * https://docs.openstack.org/api-ref/compute/?expanded=list-server-groups-detail,create-server-group-detail,show-flavor-details-detail,list-flavors-with-details-detail#list-flavors * https://docs.openstack.org/api-ref/compute/?expanded=list-server-groups-detail,create-server-group-detail,show-flavor-details-detail#list-flavors
448ef7e to
2d87d05
Compare
|
@spjmurray I also missed this PR, but I'm not a maintainer. @mandre @pierreprinetti have all the cards |
|
No wonder they always win at poker! |
|
Thanks for your eyes, I'm not surprised with the time, I've got my fingers in CAPO, openstack-cloud-controller-manager, argocd, and my own platform to boot. Hard to juggle everything at once! |
|
@pierreprinetti can I volunteer to join the gophercloud maintainers list? |
|
The only reason #2561 has not yet merged is that it's a breaking change, and we needed a new branch for merging such changes. From the output of We might be able to start merging breaking changes now that we've branched out v1. |
|
I'm closing this PR as it's indeed duplicating #2561. |
|
Sounds reasonable, can't expect everyone to want to use DeepEqual everywhere. I will await its merger! |
Add the "extra_specs" field in compute flavors, that can avoid another lookup. As documented it's for microversions 2.61 and greater. Documented here:
Prior to starting a PR, please make sure you have read our
contributor tutorial.
Prior to a PR being reviewed, there needs to be a Github issue that the PR
addresses. Replace the brackets and text below with that issue number.
Fixes #2497
Links to the line numbers/files in the OpenStack source code that support the
code in this PR:
See commit message... seems I jumped the gun 😸