| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-04 | |||
| 19:09:45 | sean-k-mooney | mriedem: i get why we might want to do this to prevent raising minimums higher then needed but i think we are also losing that auto blacklisting of specific versions e.g. x>1.0,x!=1.1 | |
| 19:09:48 | dansmith | so that things like swift can continue supporting the older versions of some libraries they have because of workarounds or lack of actual requirements for the newer one | |
| 19:10:23 | mriedem | i just see that it's throwing people off e.g. https://review.openstack.org/#/c/554679/ | |
| 19:11:36 | mriedem | and https://review.openstack.org/#/c/558254/ | |
| 19:11:37 | dansmith | just yet another change in process | |
| 19:11:42 | mriedem | which is actually < than what's in g-r | |
| 19:11:44 | openstackgerrit | Tyler Blakeslee proposed openstack/nova master: Add __repr__ for NovaException https://review.openstack.org/555812 | |
| 19:11:52 | sean-k-mooney | oh so in the scale io case they had expect the version bump but the proposal bot is not making it | |
| 19:12:05 | openstackgerrit | Merged openstack/nova master: [placement] api-ref: Fix parameters https://review.openstack.org/558142 | |
| 19:12:18 | mriedem | i'm not sure why we'd even have g-r then | |
| 19:12:50 | sean-k-mooney | mriedem: i gues g-r keeps the minium for all pojects and nova requirments.txt has minium for nova | |
| 19:13:28 | cdent | mriedem: upper constraints remains the one true thing, even without a globally sync requirements.txt, maybe? | |
| 19:13:40 | mriedem | which means we could bump our minimum required version in nova for a library, but not care about bumping it in g-r | |
| 19:13:57 | dansmith | we still have to be below u-c | |
| 19:13:57 | sean-k-mooney | so if you dont deploy something that need the newer requirement you can still use version below g-r. that said our gate jobs always run with whatever is allowed by u-c | |
| 19:14:11 | mriedem | u-c bumps are automated | |
| 19:14:14 | mriedem | and merged in batches | |
| 19:14:25 | openstackgerrit | Eric Young proposed openstack/nova master: Support extending attached ScaleIO volumes https://review.openstack.org/554679 | |
| 19:14:31 | dansmith | my feeling is that we'll still have some things where you can't actually use nova at the version pinned by cinder for a given thing | |
| 19:15:03 | mriedem | yeah. forget i asked. i need to find a hole to put my head. | |
| 19:15:57 | mriedem | the foot of snow we got yesterday doesn't help | |
| 19:16:15 | dansmith | better you than me | |
| 19:19:17 | kashyap | [nag] Since I'm awake at this random hour in front of the screen, might as well point out this if you are looking for something to +A: https://review.openstack.org/#/c/534384/ | |
| 19:19:27 | kashyap | It already has a +2 | |
| 19:20:29 | smcginnis | mriedem: I don't think g-r matters anymore and is probably going away at some point. | |
| 19:20:40 | smcginnis | Things just need to be between lower-constraint and upper-constraint. | |
| 19:21:52 | sean-k-mooney | smcginnis: isnt g-r the lower-constraint | |
| 19:22:08 | sean-k-mooney | smcginnis: or at least openstack wide lower-constraint | |
| 19:22:11 | smcginnis | sean-k-mooney: No, there is a new lower-constraint.txt file. | |
| 19:22:25 | smcginnis | sean-k-mooney: Well, I guess you could call that the new g-r. | |
| 19:23:01 | smcginnis | It's slightly different though: https://github.com/openstack/requirements/blob/master/lower-constraints.txt | |
| 19:23:13 | sean-k-mooney | smcginnis: oh ok is that updated automatically some how? just not sure what the delta is between it and g-r | |
| 19:23:36 | sean-k-mooney | oh its expcitly === with no ranges | |
| 19:24:12 | smcginnis | Right. It's saying "the minimum required is exactly this" rather than "it needs to be above this version, but not this one, etc." | |
| 19:24:15 | sean-k-mooney | so in theroy we could use it to test with minium supported version | |
| 19:24:27 | smcginnis | sean-k-mooney: Yep, I think that's the plan. | |
| 19:25:21 | smcginnis | It's a bit of a long read, but Doug wrote out the whole plan here: http://lists.openstack.org/pipermail/openstack-dev/2018-March/128352.html | |
| 19:26:29 | sean-k-mooney | smcginnis: oh good to know. i basically assume that since we only tested with what upperconstraties allowed that anything lower might work but not worth the heart ache of finding out | |
| 19:27:09 | smcginnis | sean-k-mooney: Hah, yeah. And I think in a lot of cases, our lower bound didn't/doesn't accurately reflect what really is the minimum required. | |
| 19:27:18 | smcginnis | This makes it plausible to have a test that can verify that. | |
| 19:28:16 | sean-k-mooney | smcginnis: well the distros. esspcially centos et al would be happy with knowing that a minium is actully checked before they start intergrating | |
| 19:28:28 | sean-k-mooney | ill give the ml post a read thanks | |
| 19:28:54 | smcginnis | sean-k-mooney: Pour yourselve a nice cup of tea first - it will take a while. ;) | |
| 19:52:45 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Block swapping to an encrypted volume when using QEMU to decrypt https://review.openstack.org/544238 | |
| 19:59:44 | openstackgerrit | Chris Dent proposed openstack/nova master: Move test_report_client out of placement namespace https://review.openstack.org/558911 | |
| 20:20:03 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Fix incorrect exception import https://review.openstack.org/558916 | |
| 20:20:35 | cdent | efried, mriedem, dansmith : that's ^ a fun little bug fix that would be nice to have | |
| 20:24:06 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Add --enable and --disable options to nova-manage update_cell https://review.openstack.org/555416 | |
| 20:24:57 | mriedem | cdent: we can't even have a simple unit test for that if gabbi won't cover it? | |
| 20:25:35 | cdent | mriedem: we can have a unit test for it but it would be...very mocky | |
| 20:25:49 | mriedem | that's fine, it's testing error handling | |
| 20:25:52 | melwitt | yeah, I was about to ask that. I'm not seeing unit tests in the tree for the aggregate handler, guess there hasn't been need of one yet | |
| 20:26:12 | cdent | melwitt, mriedem : as a general rule we haven't done unit tests for the handler code | |
| 20:26:16 | melwitt | that is, gabbit tests cover most of it | |
| 20:26:20 | melwitt | gabbi | |
| 20:26:22 | mriedem | cdent: i know | |
| 20:26:25 | mriedem | but... | |
| 20:26:30 | mriedem | clearly there is a need for it in some cases | |
| 20:26:36 | melwitt | aye | |
| 20:26:49 | cdent | I'm not saying I'm agin it, just that it hasn't happened yet | |
| 20:27:02 | mriedem | you can be a trailblazer here | |
| 20:27:19 | cdent | I was hoping to go to sleep instead | |
| 20:27:30 | mriedem | blaze that treasure trail in the morning | |
| 20:27:44 | mriedem | or lose sleep over it tonight :) | |
| 20:28:04 | cdent | I will lose sleep over trying to generate caring | |
| 20:29:00 | cdent | mriedem: so I can both think about it and not think about it, what is that you're hoping for here? | |
| 20:29:43 | mriedem | i'm -1 without a unit test | |
| 20:29:54 | mriedem | which can be dealt with whenever you feel like it i guess | |
| 20:30:00 | cdent | a test that confirms that the handler raises a 409 when it seens a ConcurrentUpdate, or that a ConcurrentUpdate happens when there is an increment generation failure that casues a ConcurrentUpdate | |
| 20:30:18 | mriedem | i'm fine with the former | |
| 20:30:20 | mriedem | seems easy enough | |
| 20:30:25 | cdent | one of the reasons we haven't done it in the past is because the answer to that ^ is unclear | |
| 20:30:35 | melwitt | +1 on the former | |
| 20:30:49 | mriedem | making sure we don't spew 500 out of the REST API is a simple enough thing to say is a good test | |
| 20:31:03 | cdent | hmmm. Would that even have caught this particular problem? | |
| 20:31:35 | mriedem | the exception moved, | |
| 20:31:37 | cdent | I suppose so, as the unit test itself would have had an import error when it tried to side effet | |
| 20:31:37 | mriedem | so yes it should | |
| 20:31:42 | melwitt | is ConcurrentUpdate a base nova exception too? | |
| 20:31:47 | mriedem | no | |
| 20:31:47 | cdent | not any more | |
| 20:31:48 | mriedem | it moved | |
| 20:31:58 | melwitt | okay, so you'd think it would blow up there | |
| 20:37:11 | melwitt | my concern is just let's patch that test coverage gap since we know it's there. so if that path breaks in the future, we'll catch it. whether that happens now or in a follow up is fine IMHO but I think it's worth doing | |
| 20:43:39 | cdent | I get the concern, my reluctance is mostly because we've done a good job of avoid mock madness in the tests associated with placement | |
| 20:43:59 | openstackgerrit | Merged openstack/nova master: network: add command to configure trusted mode for VFs https://review.openstack.org/458513 | |
| 20:44:06 | cdent | I even added https://review.openstack.org/#/c/557355/ | |
| 20:48:49 | melwitt | yeah, I understand. I do really like the gabbi testing of the placement APIs. I'm just not immediately seeing another way to cover this particular testing gap | |
| 20:49:01 | openstackgerrit | Tyler Blakeslee proposed openstack/nova master: Add __repr__ for NovaException https://review.openstack.org/555812 | |
| 21:01:40 | dansmith | mriedem: melwitt cells meeting? | |
| 21:03:20 | mriedem | oh yeah | |
| 21:09:56 | cdent | efried: on https://review.openstack.org/#/c/548249/ do you remember why you are catching DBDuplicateError around set_aggregates? | |
| 21:10:06 | efried | ... | |
| 21:11:57 | cdent | efried: s/Error/Entry. That exception is handled in set_aggregates but with a pass, so I'm wondering if there's something else. it's not clear | |
| 21:12:00 | efried | cdent: Because Jay via https://review.openstack.org/#/c/548249/6/nova/api/openstack/placement/handlers/aggregate.py@100 pointed me to https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/inventory.py#L171-L174 which I copy/pasted. | |
| 21:13:33 | cdent | hrmm | |
| 21:14:47 | efried | cdent: Looking through a little bit, it's possible it's not necessary. | |
| 21:14:57 | cdent | will leave a NOTE next to it for the time being | |
| 21:15:04 | efried | ight. | |
| 21:23:38 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add nova-status check for ironic flavor migration https://review.openstack.org/527541 | |