Flavor Extra Specs Create by dklyle · Pull Request #689 · gophercloud/gophercloud

dklyle · 2017-12-21T20:18:04Z

Links to the line numbers/files in the OpenStack source code that support the
code in this PR:

API reference:
https://developer.openstack.org/api-ref/compute/#create-extra-specs-for-a-flavor

Nova implementation:
https://github.com/openstack/nova/blob/stable/pike/nova/api/openstack/compute/flavors_extraspecs.py#L61

Schema:
https://github.com/openstack/nova/blob/stable/pike/nova/api/openstack/compute/schemas/flavors_extraspecs.py#L24
https://github.com/openstack/nova/blob/stable/pike/nova/api/validation/parameter_types.py#L363-L371

coveralls · 2017-12-21T20:25:29Z

Coverage increased (+0.07%) to 72.895% when pulling f0059a6 on dklyle:create-flavor-extra-specs into cf81d92 on gophercloud:master.

theopenlab-ci · 2017-12-21T20:53:29Z

Build succeeded.

gophercloud-unittest http://logs.openlabtesting.org/logs/89/689/f0059a69e7c882f27fc5968892511d2e17da5e18/check/gophercloud-unittest/cdfa2fd/ : SUCCESS in 1m 11s
gophercloud-acceptance-test http://logs.openlabtesting.org/logs/89/689/f0059a69e7c882f27fc5968892511d2e17da5e18/check/gophercloud-acceptance-test/aadd5f2/ : SUCCESS in 34m 54s

coveralls · 2017-12-21T22:56:14Z

Coverage increased (+0.07%) to 72.921% when pulling 2e8433e on dklyle:create-flavor-extra-specs into 7b1b877 on gophercloud:master.

theopenlab-ci · 2017-12-21T23:33:01Z

Build succeeded.

gophercloud-unittest http://logs.openlabtesting.org/logs/89/689/2e8433e6868e39f960dc6238749cb977b5f82b20/check/gophercloud-unittest/ddbe05b/ : SUCCESS in 1m 04s
gophercloud-acceptance-test http://logs.openlabtesting.org/logs/89/689/2e8433e6868e39f960dc6238749cb977b5f82b20/check/gophercloud-acceptance-test/75a3d00/ : SUCCESS in 43m 40s

jtopjian

@dklyle A few minor nits:

newlines in the docs
Making the IDFromName to be the bottom-most function (not that it's specifically about the IDFromName but more keeping all of the "core" types, interfaces, and methods in the same area of the file.

And some other changes:

Can you add a CreateExtraSpecsOptsBuilder interface. Update would have an UpdateExtraSpecsOptsBuilder and it's quite possible delete won't since it looks like the Delete call doesn't have a body?

Finally, could you add a GetExtraSpec to the acc test since it's now possible to retrieve a newly created spec?

Let me know if you have any questions.

jtopjian · 2017-12-21T23:21:59Z

openstack/compute/v2/flavors/doc.go

 		panic(err)
 	}
+
+Example to Create Extra Specs for a Flavor


nit: Newline below this line.

jtopjian · 2017-12-21T23:22:17Z

openstack/compute/v2/flavors/doc.go

+
+	fmt.Printf("%+v", createdExtraSpecs)
+
+Example to Get Extra Specs for a Flavor


nit: Newline below this line.

theopenlab-ci · 2017-12-22T00:36:55Z

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 · 2017-12-22T00:42:11Z

I made the suggested changes. The only one I find questionable is the CreateExtraSpecsOptsBuilder. Mainly because the json construct in ToExtraSpecsCreateMap is setting extra_specs. This particular API doesn't leave a higher level object to hang additional parameters off of. So the usage of CreateExtraSpecsOptsBuilder would be to put values into ExtraSpecOpts which is only a map[string]string. I imagine one could create some special values in the map to add extra parameters, but I'm not sure that would change the structure of the map. As such, I find this addition questionable. Maybe a more elaborate change is required around building the opts. Not sure.

coveralls · 2017-12-22T00:51:10Z

Coverage increased (+0.01%) to 72.921% when pulling be3fd78 on dklyle:create-flavor-extra-specs into c2cafb4 on gophercloud:master.

theopenlab-ci · 2017-12-22T01:23:02Z

Build succeeded.

gophercloud-unittest http://logs.openlabtesting.org/logs/89/689/be3fd7845c1928cbc5bbe289f2e39f5dec2e7278/check/gophercloud-unittest/93aab21/ : SUCCESS in 2m 39s
gophercloud-acceptance-test http://logs.openlabtesting.org/logs/89/689/be3fd7845c1928cbc5bbe289f2e39f5dec2e7278/check/gophercloud-acceptance-test/f14b926/ : SUCCESS in 37m 40s

jtopjian · 2017-12-22T01:29:28Z

@dklyle Yeah, the standard comment we leave for the Builder interfaces doesn't make a lot of sense in this case. You're right - there's not much one can do in the way of adding additional parameters here when they can just pass them all in the original map.

The ToExtraSpecsCreateMap function can be used for other things beyond adding additional data, though. For example, it can validate the specs being requested so they adhere to in-house rules or something like that. In a way, the interface and function are more of a hook.

edit: Also, using interfaces here matches what's being done with the servers' Metadata.

More to that: the metadata ToMetadatumCreateMap has a built-in check to make sure only 1 metadata item is being passed. Perhaps some private cloud has the same requirement for ExtraSpecs, so they would implement this kind of check through the ToExtraSpecsCreateMap.

dklyle · 2017-12-22T02:32:07Z

Fair enough. That's an explanation I can buy. I think the singular set of a specific key may need the same limit of only 1 item.

jtopjian · 2017-12-22T03:02:16Z

I think the singular set of a specific key may need the same limit of only 1 item.

It looks like this might be required for the Update call, although it looks like the function will accept an arbitrary amount of specs and as long as the key in the URL matches at least one spec, everything else will pass. But I haven't tested that theory.

If that's true, though, I would side on passing the entire map to Nova and letting it do the validation.

This all looks good, though. Thank you for the updates and definitely thank you for pushing back on suggestions :)

jtopjian · 2017-12-22T03:14:43Z

@dklyle Let's finish off ExtraSpecs before RemoveAccess - that should minimize the amount of toes being stepped on :)

No rush, though.

) * Networking V2: Update port DHCP options in one API call * Allow DHCP opts value update

dklyle force-pushed the create-flavor-extra-specs branch from f0059a6 to 2e8433e Compare December 21, 2017 22:48

jtopjian mentioned this pull request Dec 21, 2017

Flavor Extra Specs: List / Get #686

Merged

jtopjian requested changes Dec 21, 2017

View reviewed changes

dklyle force-pushed the create-flavor-extra-specs branch from 2e8433e to d2ff973 Compare December 22, 2017 00:36

Flavor Extra Specs Create

be3fd78

dklyle force-pushed the create-flavor-extra-specs branch from d2ff973 to be3fd78 Compare December 22, 2017 00:44

jtopjian approved these changes Dec 22, 2017

View reviewed changes

jtopjian merged commit 05116c7 into gophercloud:master Dec 22, 2017

jtopjian mentioned this pull request Dec 22, 2017

Compute v2: Flavor Extra Specs #540

Closed

5 tasks

dklyle mentioned this pull request Jan 10, 2018

Flavor Extra Spec Update #712

Merged

dklyle deleted the create-flavor-extra-specs branch January 11, 2018 05:08

cardoe pushed a commit to cardoe/gophercloud that referenced this pull request Aug 27, 2020

Networking V2: Update port DHCP options in one API call (gophercloud#689

acd172d

) * Networking V2: Update port DHCP options in one API call * Allow DHCP opts value update


		fmt.Printf("%+v", createdExtraSpecs)

		Example to Get Extra Specs for a Flavor

Conversation

dklyle commented Dec 21, 2017

Uh oh!

coveralls commented Dec 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theopenlab-ci bot commented Dec 21, 2017

Uh oh!

coveralls commented Dec 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theopenlab-ci bot commented Dec 21, 2017

Uh oh!

jtopjian left a comment

Choose a reason for hiding this comment

Uh oh!

jtopjian Dec 21, 2017

Choose a reason for hiding this comment

Uh oh!

dklyle Dec 22, 2017

Choose a reason for hiding this comment

Uh oh!

jtopjian Dec 21, 2017

Choose a reason for hiding this comment

Uh oh!

dklyle Dec 22, 2017

Choose a reason for hiding this comment

Uh oh!

theopenlab-ci bot commented Dec 22, 2017

Uh oh!

dklyle commented Dec 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Dec 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theopenlab-ci bot commented Dec 22, 2017

Uh oh!

jtopjian commented Dec 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dklyle commented Dec 22, 2017

Uh oh!

jtopjian commented Dec 22, 2017

Uh oh!

jtopjian commented Dec 22, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented Dec 21, 2017 •

edited

Loading

coveralls commented Dec 21, 2017 •

edited

Loading

dklyle commented Dec 22, 2017 •

edited

Loading

coveralls commented Dec 22, 2017 •

edited

Loading

jtopjian commented Dec 22, 2017 •

edited

Loading