| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-22 | |||
| 16:04:21 | mriedem | sdague: cdent: so to be clear, we can remove the body for this https://developer.openstack.org/api-ref/compute/?expanded=disable-scheduling-for-a-compute-service-detail#disable-scheduling-for-a-compute-service | |
| 16:04:24 | mriedem | which then makes it a POST | |
| 16:04:41 | mriedem | but we'd still have a body for https://developer.openstack.org/api-ref/compute/?expanded=log-disabled-compute-service-information-detail#log-disabled-compute-service-information | |
| 16:04:48 | mriedem | so does that become POST or remain PUT? | |
| 16:05:09 | mriedem | also, does the response affect which method it is in any way? | |
| 16:05:12 | sdague | mriedem: is that changing urls as well? | |
| 16:05:16 | mriedem | since these all return a response body | |
| 16:05:19 | mriedem | sdague: yes | |
| 16:05:26 | sdague | mriedem: honestly, I think from the principle of least surprise | |
| 16:05:43 | sdague | if you are going to GET the same url as the modification, PUT is fine | |
| 16:05:54 | sdague | if it's a different URL, POST is better | |
| 16:05:57 | mriedem | all 4 PUTs here would change the URL https://developer.openstack.org/api-ref/compute/#compute-services-os-services | |
| 16:06:08 | sdague | mriedem: which will have GET urls still? | |
| 16:06:23 | mriedem | the only GET we have is to list services | |
| 16:06:29 | sdague | yeh, so I'd make them POST | |
| 16:06:46 | sdague | if the urls are changing anyway | |
| 16:07:06 | mriedem | let me make sure we don't have GET /os-services/{service_id} | |
| 16:07:24 | mriedem | yeah we don't | |
| 16:07:40 | mriedem | ok so change all 4 PUTs to POSTs and modify the URL to include the {service_id} | |
| 16:07:48 | mriedem | and drop host/binary from the POST body | |
| 16:07:54 | mriedem | since those are replaced by the service_id | |
| 16:08:28 | mriedem | i'll make those changes to the spec | |
| 16:08:35 | mriedem | thanks y'all | |
| 16:08:55 | cdent | I was under the impression we didn't want to make that breadth of change, but since we are willing, that sounds like the right way to go | |
| 16:09:23 | sdague | actually, I'm going to raise a bigger question | |
| 16:09:52 | sdague | why isn't this just a PUT/PATCH of /os-services/{service_id} | |
| 16:10:20 | sdague | every one of these urls is really an attribute in the response | |
| 16:10:22 | mriedem | with a body with the action to perform? | |
| 16:10:42 | mriedem | or PUT with 'status' in the req body? | |
| 16:10:44 | cdent | that's what I think is the long term right way, but I thought we didn't want that because people did want the attribute free way of doing stuff | |
| 16:10:48 | sdague | http://paste.openstack.org/show/603789/ | |
| 16:11:11 | sdague | mriedem: yeh, just like how we can update the description in a server | |
| 16:12:42 | mriedem | we could do that too | |
| 16:12:47 | cdent | the debate surrounding that kind of thing in the past has been that PUT is supposed to be a full representation, and PATCH is supposed to be a diff, not a partial representation. <- just reporting, not presenting that as my position | |
| 16:13:21 | cdent | and some people think a full rep PUT to change one thing is wasteful | |
| 16:13:24 | sdague | which would also let you do things like set {"status": "disabled", "disabled_reason": "...."} | |
| 16:13:25 | mriedem | all of these actions are actualy served by the update method in the controller anyway | |
| 16:13:26 | cdent | because there are only so many bytes in the universe | |
| 16:13:31 | mriedem | with the id being the action | |
| 16:13:36 | sdague | so, we already do PUT with partials, and I think that's fine | |
| 16:14:02 | sdague | I honestly am anti PATCH, because I think it is really for something more like what swift uses it for | |
| 16:14:07 | sdague | not for this stuff | |
| 16:14:23 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in console_auth_tokens.py https://review.openstack.org/448663 | |
| 16:14:26 | mriedem | https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/services.py#L212 is what handles all of those PUTs | |
| 16:14:30 | mriedem | the id is the action | |
| 16:14:31 | sdague | mriedem: well, it would be PUT /os-services/{service_id} ... stuff you want to change | |
| 16:14:36 | mriedem | sdague: right | |
| 16:14:47 | mriedem | so we'd have another microversioned update method, | |
| 16:14:48 | sdague | just like severs today | |
| 16:14:53 | mriedem | where id becomes the service uuid | |
| 16:15:00 | mriedem | and the body contains the things you want to change | |
| 16:15:06 | mriedem | like status or forced-down | |
| 16:15:47 | sdague | yeh | |
| 16:16:11 | sdague | especially because those things are reflected in the representation when you get it | |
| 16:16:24 | mriedem | i'd kind of like to split that into a separate spec/microversion, but doing that gets weird when we also want to change the url | |
| 16:16:44 | sdague | yeh, I think it should all change at once, otherwise it's just painful for users | |
| 16:16:46 | mriedem | so probably can't split | |
| 16:16:48 | mriedem | right | |
| 16:17:02 | mriedem | since we don't test any of this in tempest, i worry | |
| 16:17:10 | mriedem | but anyway | |
| 16:17:51 | cdent | someday | |
| 16:18:26 | mriedem | it's just a lot of cases for the single PUT to handle, but we'll see | |
| 16:18:37 | mriedem | plus there is already another microversion in this flow already, 2.11 | |
| 16:18:38 | mriedem | for forced-down | |
| 16:18:39 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in cloudpipe.py https://review.openstack.org/448271 | |
| 16:18:40 | sdague | we don't test this stuff in tempest because it would break the world | |
| 16:18:44 | mriedem | sdague: i know | |
| 16:18:49 | mriedem | i know why, i'm just saying | |
| 16:18:50 | mriedem | you know | |
| 16:18:57 | sdague | you can't start randomly turning off the compute service in the middle of tests | |
| 16:19:03 | sdague | it's all testable on the functional side | |
| 16:19:07 | mriedem | yes i know | |
| 16:19:25 | mriedem | can't you just let me worry | |
| 16:22:32 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in certificates.py https://review.openstack.org/448267 | |
| 16:24:01 | mriedem | replied in https://review.openstack.org/#/c/447149/1/specs/pike/approved/service-hyper-uuid-in-api.rst@148 and left examples of what the single PUT would look like | |
| 16:24:11 | mriedem | i can detail how a single PUT replaces the existing 4 PUT actions | |
| 16:25:50 | openstackgerrit | Gábor Antal proposed openstack/nova master: Add json style checking for sample notifications https://review.openstack.org/443677 | |
| 16:30:11 | cfriesen | maybe an odd question....when we download a qcow2-formatted image we convert it to raw before storing it under "_base". Is there a way to leave it as-is? I'm pretty sure qemu understands using qcow2 images as backing files. | |
| 16:30:19 | cfriesen | Or does that cause a performance hit at runtime? | |
| 16:53:24 | johnthetubaguy | cfriesen: I think there is a spec about that, looking at similar things with ploop | |
| 16:56:30 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in deferred_delete.py https://review.openstack.org/448681 | |
| 16:56:48 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in create_backup.py https://review.openstack.org/448682 | |
| 16:57:00 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in consoles.py https://review.openstack.org/448683 | |
| 16:57:19 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in console_output.py https://review.openstack.org/448684 | |
| 17:06:56 | openstackgerrit | Thomas Stewart proposed openstack/nova master: Add lan9118 as valid nic for hw_vif_model property for qemu https://review.openstack.org/393489 | |
| 17:37:42 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Use uuids in services and os-hypervisors APIs (spec) https://review.openstack.org/447149 | |
| 17:37:54 | mriedem | johnthetubaguy: sdague: cdent: dansmith: alex_xu: ^ has the updates we talked about earlire | |
| 17:37:56 | mriedem | *earlier | |
| 17:39:19 | cdent | ✔ | |
| 18:07:07 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Reserve NUMA nodes with PCI devices attached https://review.openstack.org/364468 | |
| 18:08:53 | mriedem | johnthetubaguy: you can drop the -2 on https://review.openstack.org/#/c/379524/ now, the bp is approved | |
| 18:21:38 | mriedem | dansmith: i've got a pickle, i'm removing the aggregate.uuid migrate on load in the object https://review.openstack.org/#/c/448579/ | |
| 18:21:49 | mriedem | that's failing because there is a test for the online data migration which uses the object, | |
| 18:22:04 | mriedem | and because i've removed that code, the online data migration blows up since it can't use the object anymore | |
| 18:22:16 | dansmith | ...right | |
| 18:22:29 | dansmith | we can remove the migration now | |
| 18:22:31 | mriedem | i can't remove the online data migration thing from nova-manage can i? | |
| 18:22:33 | mriedem | oh | |
| 18:22:45 | mriedem | so i guess if you jump from what, mitaka to pike, well, sorry | |
| 18:22:46 | mriedem | derp | |