| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-21 | |||
| 14:26:02 | jroll | I promised I'd help with placement stuff, yay I did a thing | |
| 14:26:22 | bauzas | at least we discussed how scary was the IronicHostManager :) | |
| 14:26:35 | bauzas | now mriedem could just throw it under the bus too :p | |
| 14:26:42 | jroll | that's drilled into my head for years now | |
| 14:27:01 | bauzas | FWIW, I'm even not convinced by HostState objects | |
| 14:27:10 | sdague | sfinucan: on https://review.openstack.org/#/c/430912 ... if we put an idempotent id in the commit message, can we put the full one in there? If it's full gerrit makes it a link, the partial isn't | |
| 14:27:37 | bauzas | anyway, /me disappearing b/c paternal duties | |
| 14:28:16 | sfinucan | sdague: Weird. Partial commit refs _do_ work. I'll fix that now | |
| 14:28:46 | sdague | sfinucan: yeh, but not Idempotent ids | |
| 14:28:53 | sdague | I think it's gerrit being conservative | |
| 14:28:54 | sfinucan | sdague: It does in https://review.openstack.org/#/c/430911/5 though? | |
| 14:29:12 | mriedem | rpodolyaka: you've got to sort the projects.yaml file https://review.openstack.org/#/c/448115/1 | |
| 14:29:22 | sdague | sfinucan: only 7 in the unlinked one? | |
| 14:29:33 | sdague | it's still probably good form to do the whole thing though | |
| 14:30:13 | mriedem | rpodolyaka: let me know when we get to the point of needing to setup a launchpad project for bug tracking | |
| 14:30:21 | alex_xu | cdent: what is your expect when no content-length and content-type, 400 or 415? | |
| 14:30:25 | sfinucan | sdague: Aye, it does work with 8 chars | |
| 14:31:12 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Ignore 'allow_same_net_traffic' for port filters https://review.openstack.org/430912 | |
| 14:31:55 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Deprecate 'allow_same_net_traffic' https://review.openstack.org/430913 | |
| 14:32:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Final cleanups in conf/network https://review.openstack.org/431023 | |
| 14:32:35 | sfinucan | sdague: Thanks for reviewing those :) | |
| 14:32:42 | cdent | alex_xu: 400 | |
| 14:32:53 | cdent | 415 is only for when the value of content-type is not the one desired | |
| 14:33:07 | alex_xu | cdent: but the https://review.openstack.org/447625 will return 415 | |
| 14:33:08 | rpodolyaka | mriedem: oops, will fix that now | |
| 14:33:31 | openstackgerrit | Béla Vancsics proposed openstack/nova master: Transform instance.trigger_crash_dump notification https://review.openstack.org/396225 | |
| 14:34:00 | rpodolyaka | mriedem: hmm, I already created one - https://bugs.launchpad.net/placement-osc-plugin . Was that too early? | |
| 14:34:39 | cdent | alex_xu: sorry I misunderstood what you meant: I thought you mean no content-type and no content-length. For no content-length but with a content-type then 415 (assuming the content-type is wrong) | |
| 14:35:05 | alex_xu | cdent: yes, I mean no content-type and no content-length | |
| 14:36:16 | cdent | alex_xu: okay, ideally that would be a 400, but I've allowed it to fall through to a 415 because we don't want to do either of the following: | |
| 14:36:28 | cdent | * check for the presence of a body (before actually reading it) | |
| 14:36:42 | cdent | * limit things based on which method, since GET could send a body | |
| 14:36:49 | mriedem | rpodolyaka: less work for me | |
| 14:36:53 | rpodolyaka | :) | |
| 14:37:56 | mriedem | mdbooth: on https://review.openstack.org/#/c/441989/ i noticed there was no unit test, which, whatever, but if having a method defined is required to actually have things work, what are your thoughts on using ABCs in the imagebackend code? | |
| 14:37:59 | cdent | alex_xu: a thing to keep in mind is that is hard, without explicit intent, to not send a content-length header when you have a body | |
| 14:38:18 | mriedem | so if a method is required, like get_model, then define it as an abstractmethod | |
| 14:38:36 | cdent | alex_xu: we could change things back to check for the request method, but since we've had discussions about GET using bodies, I didn't want to build that in | |
| 14:38:53 | mriedem | i guess Image is an ABC | |
| 14:39:15 | mriedem | but get_model() isn't marked as an abstractmethod? that seems wrong | |
| 14:39:21 | cdent | alex_xu: we can change it like you suggest (all in the decorator) but that doesn't feel as robust to me | |
| 14:39:35 | mdbooth | mriedem: I vaguely recall considering that when I looked at it, but we don't use ABCs very consistently so I wasn't that bothered IIRC | |
| 14:39:42 | mriedem | mdbooth: i'll push a change quick | |
| 14:39:50 | alex_xu | cdent: sorry, I didn't get what relationship with GET using bodies | |
| 14:40:06 | mriedem | so when MyCoolImageEnterpriseBackend shows up, we don't have the same issue | |
| 14:40:43 | jroll | EmilienM: I rechecked your test patch, btw | |
| 14:40:49 | EmilienM | thx | |
| 14:40:55 | EmilienM | I'm still in mtg | |
| 14:41:15 | jroll | no worries, just a heads up :) | |
| 14:41:23 | alex_xu | emm...actually, i didn't get why that two points related to my suggestion. I may miss something | |
| 14:43:22 | edleafe | cdent: (reading scrollback) So no content-type with no content is not OK? | |
| 14:43:59 | cdent | alex_xu: there's two things going on in the patch: One is making sure that if there is no body (thus the content-length header is either 0 or not there) then a PUT or POST can work. Two is making sure that if there is a body (content-length header is present and has a value) then the content-type header is present, no matter what the request method is. | |
| 14:44:02 | cdent | edleafe: ^ | |
| 14:45:24 | cdent | Item number two should be true for every request and should lead to a 400 if not the case. Thus having it as part of a decorator is not ideal. | |
| 14:46:03 | cdent | (unless it was part of the wsgify decorator, but that's essentially the same thing as where it is now) | |
| 14:46:04 | edleafe | cdent: I was reading alex_xu's comment at http://p.anticdent.org/1sVC and your reply | |
| 14:46:45 | cdent | edleafe: if there is a body, and there is either no content-type or no content-length, that's a 400 | |
| 14:47:17 | cdent | edleafe: but we only have the content-length header as a reasonable proxy for knowing if there is a body | |
| 14:47:21 | edleafe | cdent: a body with no content-length? | |
| 14:47:21 | openstackgerrit | Sujitha proposed openstack/nova master: Change os-server-tags default policy https://review.openstack.org/396420 | |
| 14:48:22 | cdent | edleafe: a healthy client would never do that, but if, for example, you telnet to port 80 and write a PUT it's easy to do | |
| 14:48:37 | cdent | also, if chunked encoding is being used, all bets are off | |
| 14:48:45 | cdent | but we don't use that, so that's a relief | |
| 14:50:43 | cdent | alex_xu, edleafe: does that help clear things up or just make it more muddy? sadly we can't protect against all cases of "telnet to port 80" but we can cover some. | |
| 14:51:22 | cdent | on the other hand it is entirely possible I'm putting too much trust in the existing gabbi tests, but since they pass, I feel pretty okay | |
| 14:51:50 | alex_xu | I think i get your point now | |
| 14:55:30 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add description to policies in assisted_volume_snapshots.py https://review.openstack.org/445238 | |
| 14:55:44 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Add description to policies in admin_password.py https://review.openstack.org/443957 | |
| 14:55:45 | edleafe | cdent: you mean there are other ways to use OpenStack other than telnet? | |
| 14:56:16 | rfolco | mriedem, edleafe: re: first pike bp implemented: nice! thanks to edleafe, cdent and jaypipes for great guidance :) | |
| 14:56:17 | cdent | edleafe: only if you pass the awkward test | |
| 14:56:35 | cdent | \o/ rfolco | |
| 14:59:00 | alex_xu | if we check the presence the body, that means we have to read the body first | |
| 14:59:05 | jaypipes | rfolco: and now you can submit a patch that changes the scheduler/report client to use DELETE /r-p/{uuid}/inventories :) | |
| 14:59:31 | alex_xu | that is waste, that is the reason cdent didn't want to check that | |
| 14:59:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: libvirt: mark some Image backend methods as abstract https://review.openstack.org/448142 | |
| 14:59:37 | mriedem | mdbooth: see how that suits you ^ | |
| 15:00:08 | cdent | alex_xu: yes, besides being a waste, it also limits some of the options for future handling | |
| 15:00:33 | alex_xu | cdent: what options? | |
| 15:00:46 | mdbooth | mriedem: WFM | |
| 15:00:49 | rfolco | jaypipes, noted | |
| 15:00:53 | jroll | cdent: edleafe: I'd love to start on this spec today, if you want to give it a quick skim before I do: https://etherpad.openstack.org/p/nova-ironic-flavor-transition-plan | |
| 15:01:08 | cdent | handling streams | |
| 15:01:14 | cdent | sorry phone | |
| 15:01:51 | edleafe | jroll: will get to it in a bit. Meeting Hell Morning and all... | |
| 15:02:15 | jroll | dansmith: mriedem: looks like most of the placement crew is happy with this flavor extra specs, spec, when you have a few: https://review.openstack.org/#/c/446570/ | |
| 15:02:17 | jroll | edleafe: no rush | |
| 15:07:28 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop using mox stubs in nova/tests/unit/cells/fakes.py https://review.openstack.org/342043 | |
| 15:08:32 | alex_xu | cdent: +2 on the patch, thanks for explain all of that to me patiently | |
| 15:12:27 | bauzas | jroll: dansmith: FWIW, just reviewing https://review.openstack.org/#/c/446570/2/specs/pike/approved/custom-resource-classes-in-flavors.rst now | |
| 15:12:54 | jroll | bauzas: woohoo | |
| 15:13:34 | cdent | alex_xu: thanks for asking so many questions and being so thourough. is great. | |
| 15:13:56 | mriedem | jroll: starred | |
| 15:16:40 | mriedem | sdague: alex_xu: oomichi: i put this on the api subteam meeting agenda, but would be nice to get your feedback, or at least start thinking about this before then https://review.openstack.org/#/c/447149/1/specs/pike/approved/service-hyper-uuid-in-api.rst@148 | |
| 15:17:08 | mriedem | alex_xu: i know it's late for you so don't worry too much about it today | |
| 15:17:20 | cdent | jroll: I dunno, after telling you to change that 0 to a 1, I am _completely_ exhausted. | |
| 15:17:25 | mriedem | just lose a *little* bit of sleep :) | |
| 15:19:41 | alex_xu | mriedem: got it, I will check it tomorrow morning | |
| 15:24:16 | dane-fichter | johnthetubaguy: got a second to talk about the trusted flavors spec? | |
| 15:31:29 | openstackgerrit | Sujitha proposed openstack/nova master: Add description to policies in admin_actions. https://review.openstack.org/440829 | |