Flavor Extra Specs: List / Get#686
Conversation
|
Build succeeded.
|
jtopjian
left a comment
There was a problem hiding this comment.
@dklyle Thank you for working on this!
This looks to be similar to the ListAccesses where I debated doing a plain "Get" or or a SinglePageResult because the request is "listing" things. We settled on the latter, so it's probably a good idea to also do the same here unless you see a reason not to?
The other idea I was kicking around was to make a dedicated type for ExtraSpecs. It's being done here (https://github.com/gophercloud/gophercloud/blob/master/openstack/sharedfilesystems/v2/sharetypes/results.go#L72-L74), but it's not being done for instance metadata, so I think it's fine to leave without a dedicated type.
| // ExtraSpecsResult contains the result of a call for (potentially) multiple | ||
| // key-value pairs. Call its Extract method to interpret it as a | ||
| // map[string]interface. | ||
| type ExtraSpecsResult struct { |
There was a problem hiding this comment.
if ExtraSpecsResult will only be used internally as the basis of other types, let's rename it to extraSpecsResult.
| return | ||
| } | ||
|
|
||
| func GetExtraSpecs(client *gophercloud.ServiceClient, id string, key string) (r GetExtraSpecsResult) { |
There was a problem hiding this comment.
Maybe change id here to flavorID so it isn't confused with a supposed ExtraSpecID.
There was a problem hiding this comment.
Oh, also, should this be GetExtraSpec singular?
|
|
||
| // ExtraSpecResult contains the result of a call for individual a single | ||
| // key-value pair. | ||
| type ExtraSpecResult struct { |
There was a problem hiding this comment.
Same possible lower-casing here, too.
|
Access returns a list of maps and extra-specs returns a single map. Not sure if that makes a difference, but thought I'd ask. I thought metadata looked like a closer match and tried mirroring that. But I'm certainly open to changing it. |
🤔 OK, good point. Let's keep it as-is then. |
720e8c0 to
7d47410
Compare
|
Build succeeded.
|
jtopjian
left a comment
There was a problem hiding this comment.
Just one more thing to look at.
| return | ||
| } | ||
|
|
||
| func GetExtraSpec(client *gophercloud.ServiceClient, flavorID string, key string) (r GetExtraSpecsResult) { |
There was a problem hiding this comment.
Thanks for changing the function name to singular. I think the result type also be singular (It would have to be changed in other places, too) since the "internal" type is extraSpecResult?
7d47410 to
0eb6967
Compare
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
@dklyle oops. The AddAccess PR which was just merged caused some file conflicts here. Sorry about that :( |
0eb6967 to
4dbe114
Compare
|
Build succeeded.
|
For #540
Links to the line numbers/files in the OpenStack source code that support the
code in this PR:
API reference:
List
https://developer.openstack.org/api-ref/compute/#list-extra-specs-for-a-flavor
Get
https://developer.openstack.org/api-ref/compute/#show-an-extra-spec-for-a-flavor
Nova implementation:
https://github.com/openstack/nova/blob/stable/pike/nova/api/openstack/compute/flavors_extraspecs.py#L50
https://github.com/openstack/nova/blob/stable/pike/nova/api/openstack/compute/flavors_extraspecs.py#L98
Schema:
https://github.com/openstack/nova/blob/stable/pike/nova/api/openstack/compute/schemas/flavors_extraspecs.py
https://github.com/openstack/nova/blob/stable/pike/nova/api/validation/parameter_types.py#L363-L371