Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-21
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 openstackgerrit Sujitha proposed openstack/nova master: Change os-server-tags default policy https://review.openstack.org/396420
14:47:21 edleafe cdent: a body with no content-length?
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
15:31:31 bauzas jroll: made a comment on https://review.openstack.org/#/c/446570/2
15:31:52 bauzas jroll: not sure it could be a bug, but I would appreciate some brainstorm
15:35:03 openstackgerrit Sujitha proposed openstack/nova master: Add description to policies in admin_actions. https://review.openstack.org/440829
15:37:50 openstackgerrit Sujitha proposed openstack/nova master: Add description to policies in admin_password.py https://review.openstack.org/443957
15:38:28 openstackgerrit Sujitha proposed openstack/nova master: Add description to policies in agents.py https://review.openstack.org/445208
15:40:21 openstackgerrit Sandeep Pawar proposed openstack/nova master: Libvirt volume driver for Veritas HyperScale https://review.openstack.org/443951
15:40:23 jroll bauzas: resize depends on migrate being available, right?
15:40:38 jroll (which ironic can't do)
15:40:39 bauzas jroll: nova resize ?
15:40:42 jroll yes
15:40:58 bauzas I dunno, but good point, lemme verify
15:41:07 openstackgerrit Sujitha proposed openstack/nova master: Add description to policies in aggregates.py https://review.openstack.org/445230
15:41:10 jroll I'm looking, I see lots of migration code in it
15:41:16 bauzas jroll: either way, this spec doesn't only target a specific virt driver
15:41:34 bauzas so I'm supposing it can be possible
15:41:42 bauzas eg. FPGA devices with libvirt
15:41:51 jroll bauzas: your resize concerns are around the upgrade thing we do in the ironic driver, though
15:41:54 bauzas although it would require libvirt to expose it
15:42:07 jroll (afaict)

Earlier   Later