Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-21
14:23:53 EmilienM bauzas: ack, thx
14:25:03 jroll thanks bauzas
14:25:19 bauzas jroll: nah, you thanks for tackling that
14:25:52 jroll bauzas: welcome
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.

Earlier   Later