| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-25 | |||
| 16:59:22 | efried | Please note (my) review comments on older patch sets, which enumerate some of my questions/concerns. Also some in the code comments & commit message. | |
| 16:59:24 | openstackgerrit | Kaitlin Farr proposed openstack/nova master: Parse algorithm from cipher for ephemeral disk encryption https://review.openstack.org/446072 | |
| 17:00:11 | efried | johnthetubaguy Course, the "all over the place" conf opts will need to be addressed individually, but I think I have a neat approach for handling that. | |
| 17:00:27 | efried | might be a bit too "out there"... | |
| 17:02:43 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: De-complicate some of the instance delete path https://review.openstack.org/458636 | |
| 17:02:44 | openstackgerrit | Dan Smith proposed openstack/nova master: Make target_cell() yield a new context https://review.openstack.org/458635 | |
| 17:02:44 | openstackgerrit | Dan Smith proposed openstack/nova master: Move to proper target_cell calling convention https://review.openstack.org/458634 | |
| 17:02:46 | openstackgerrit | Dan Smith proposed openstack/nova master: Clean up ClientRouter debt https://review.openstack.org/444487 | |
| 17:02:47 | openstackgerrit | Dan Smith proposed openstack/nova master: DNM: Canary test against multi-cell devstack https://review.openstack.org/458537 | |
| 17:03:37 | johnthetubaguy | efried: I added some comments, not sure if they help | |
| 17:03:43 | efried | johnthetubaguy Thanks, looking... | |
| 17:03:49 | openstackgerrit | Kaitlin Farr proposed openstack/nova master: Remove deprecated keymgr code https://review.openstack.org/439855 | |
| 17:10:56 | openstackgerrit | John Garbutt proposed openstack/nova master: Add description to policies in floating_ip files https://review.openstack.org/459791 | |
| 17:37:41 | openstackgerrit | Sumedh Degaonkar proposed openstack/nova master: Adds 'sata' as a valid disk bus for qemu and kvm hypervisors https://review.openstack.org/459794 | |
| 18:14:50 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient master: 2.43: Deprecate novaclient /os-hosts usage https://review.openstack.org/459496 | |
| 18:50:23 | cfriesen | I'm updating the "flavor info in instance details" commit, and trying to figure out how to handle the new flavor dict in api-ref/source/servers.inc in terms of references to api-ref/source/parameters.yaml. Anyone have suggestions on how to handle the fact that the "flavor" field will now be a dict? | |
| 18:54:13 | mriedem | cfriesen: so this right? https://github.com/openstack/nova/blob/master/api-ref/source/parameters.yaml#L2192 | |
| 18:54:32 | mriedem | cfriesen: they are both dicts | |
| 18:54:37 | mriedem | so type is still going to be 'object' | |
| 18:54:44 | mriedem | cfriesen: you could handle it one of two ways i think, | |
| 18:55:27 | mriedem | 1. keep flavor_server and wordsmith the hell out of the description to explain that before microversion x it's a dict with id and links keys, after microversion x it's a dict with a set of other fields | |
| 18:55:59 | mriedem | 2. put max_version on flavor_server to effectively cap it, and add a new parameter with min_version to introduce the new parameter description | |
| 18:56:23 | openstackgerrit | Rick Bartra proposed openstack/nova master: Add policy granularity to the Flavors API https://review.openstack.org/449288 | |
| 18:56:29 | mriedem | cfriesen: the thing is we're probably going to want to also explain, with param descriptions, the fields in the server['flavor'] dict | |
| 18:56:56 | mriedem | those would be separate paramaters, like https://github.com/openstack/nova/blob/master/api-ref/source/parameters.yaml#L2192 | |
| 18:57:29 | mriedem | flavor_ephem_disk, flavor_disk, flavor_is_public, etc | |
| 18:57:47 | mriedem | cfriesen: just some thoughts, sdague might have opinions on this too | |
| 18:57:55 | mriedem | personally i'd lean to ward option 2 | |
| 18:57:59 | mriedem | *toward | |
| 18:59:41 | openstackgerrit | Rick Bartra proposed openstack/nova master: Add policy granularity to the Flavors API https://review.openstack.org/449288 | |
| 19:05:30 | mriedem | dansmith: i'm not sure if we want nullable=True on this build_request.tags field https://review.openstack.org/#/c/459569/1/nova/objects/build_request.py@53 | |
| 19:05:36 | mriedem | but i don't remember the rules | |
| 19:06:44 | mriedem | it's not for the instance object i guess | |
| 19:06:47 | mriedem | and this should probably mirror that | |
| 19:10:51 | cfriesen | mriedem: okay, thanks. will proceed assuming option 2 | |
| 19:13:16 | cfriesen | mriedem: can I have two different "flavor_server" parameters in parameters.yaml, one with a min_version and the other with a max_version? or do I need a separately-named parameter? | |
| 19:13:30 | mriedem | cfriesen: they have to have unique names | |
| 19:13:57 | cfriesen | mriedem: okay, so maybe something like "flavor_server_dict"? | |
| 19:14:07 | mriedem | should also be careful about changing the existing parameter in case it's used somewhere you don't expect, which isn't changing as part of your microversion | |
| 19:14:16 | mriedem | flavor_server is a dict already though | |
| 19:14:20 | cfriesen | true | |
| 19:14:24 | mriedem | assuming you have microversion 2.46, | |
| 19:14:32 | mriedem | maybe flavor_server_2_46? | |
| 19:14:49 | mriedem | which matches the min_version you'll have on it | |
| 19:15:18 | cfriesen | sure. the last one in is 2.43, are there others in progress for 2.44 and 2.45? | |
| 19:16:18 | mriedem | yes | |
| 19:16:35 | mriedem | https://review.openstack.org/#/c/455443/ | |
| 19:17:34 | cfriesen | kay...will adjust it. | |
| 19:18:47 | cfriesen | why 2_46 instead of 2.46? | |
| 19:19:15 | mriedem | can you have . in the name? | |
| 19:19:38 | cfriesen | no idea. :) | |
| 19:20:01 | cfriesen | I see this: - security_group.name: name | |
| 19:20:22 | cfriesen | I'm easy either way | |
| 19:20:59 | mriedem | security_group.name is the label in the docs | |
| 19:21:02 | mriedem | which is fine | |
| 19:21:12 | mriedem | and how we label entries within a dict/object | |
| 19:21:33 | mriedem | but within parameters.yaml i don't know if the names can have . in them | |
| 19:22:22 | cfriesen | I'll play it safe | |
| 19:23:48 | cfriesen | mriedem: as far as describing the fields in the dict, the volumeAttachment parameter in parameters.yaml just has a text description of the fields in the dict...is that the correct model to follow? | |
| 19:24:19 | mriedem | no | |
| 19:24:22 | mriedem | that's the lazy way | |
| 19:24:50 | mriedem | i recently did something with this, lemme see if i can find it | |
| 19:25:20 | openstackgerrit | Lee Yarwood proposed openstack/nova master: compute: Detach volumes on _rebuild_default_impl failure https://review.openstack.org/442105 | |
| 19:26:21 | mriedem | cfriesen: see https://review.openstack.org/#/c/449809/ | |
| 19:26:29 | mriedem | i guess you can have periods and colons and stuff in the parameter names | |
| 19:27:41 | cfriesen | mriedem: okay, will try that | |
| 19:27:47 | mriedem | that gives you https://developer.openstack.org/api-ref/compute/?expanded=show-server-details-detail#show-server-details | |
| 19:28:06 | mriedem | so os-extended-volumes:volumes_attached is the top-level array of dicts, | |
| 19:28:22 | mriedem | and each dict has os-extended-volumes:volumes_attached.id and os-extended-volumes:volumes_attached.delete_on_termination (the latter depending on microversion) | |
| 19:28:38 | mriedem | so "volumes_attached" is a kind of namespace | |
| 19:28:49 | cfriesen | mriedem: is there any way to avoid duplicating it every place that flavor_server is used? (four places in servers.inc) | |
| 19:28:51 | mriedem | and the .<field> suffix is the entry in that dict (namespace) | |
| 19:29:10 | mriedem | well, are you changing all 4 of those places? | |
| 19:29:31 | cfriesen | mriedem: yep...but they're all going to get the same dict....so I don't want to have to define the dict separately in all the places | |
| 19:30:42 | mriedem | i'm not aware of way to do that, unless you just have a single parameter and describe all of the fields in that dict within the single parameter description | |
| 19:30:55 | mriedem | i'm not totally opposed to that | |
| 19:31:00 | mriedem | sdague: oomichi: ^ thoughts? | |
| 19:31:11 | cfriesen | I don't see any precedent in parameters.yaml to define a dict | |
| 19:31:12 | mriedem | it's all .rst so you can format it nicely with bullets | |
| 19:31:52 | cfriesen | I can give it a shot....which tox test do I run to get the output of this? | |
| 19:32:02 | mriedem | tox -e api-ref | |
| 19:32:10 | mriedem | your flavor dict is going to have what, 7 fields? | |
| 19:32:15 | cfriesen | ish, yeah | |
| 19:32:24 | mriedem | https://developer.openstack.org/api-ref/compute/?expanded=show-server-details-detail,show-flavor-details-detail#id187 minus is_public and disabled | |
| 19:32:47 | mriedem | oh plus extra specs | |
| 19:32:49 | mriedem | so 8 | |
| 19:33:02 | cfriesen | yes | |
| 19:33:06 | mriedem | cfriesen: so maybe just start with a single new parameter | |
| 19:33:16 | mriedem | with the big description detailing each field within it | |
| 19:33:19 | mriedem | and we'll see how that looks | |
| 19:33:38 | cfriesen | it's too bad, because the fields are mostly already described in parameters.yaml | |
| 19:34:06 | cfriesen | but yeah, I'll give it a try | |
| 19:34:07 | mriedem | there might be a way to reference parameters, but idk | |
| 19:36:18 | openstackgerrit | Matthew Treinish proposed openstack/nova master: Remove nova-cert https://review.openstack.org/459460 | |
| 19:36:29 | mtreinish | sdague: ^^^ is that what you were looking for? | |
| 19:41:19 | oomichi | mriedem: I could not get which patch I need to review.. anyways your patch https://review.openstack.org/#/c/455443/9/nova/api/openstack/compute/rest_api_version_history.rst has strange HEAD | |
| 19:41:37 | openstackgerrit | Mathieu Gagné proposed openstack/nova master: Add ability to signal and perform online volume size change https://review.openstack.org/454322 | |
| 19:42:56 | mriedem | oomichi: gah! | |
| 19:43:03 | mriedem | oomichi: that was an easter egg for sdague :) | |
| 19:43:16 | oomichi | hehe :) | |