Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-22
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
18:22:56 dansmith ...for this and many other reasons
18:23:18 mriedem ok i retract the pickle

Earlier   Later