| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-09 | |||
| 16:51:46 | dansmith | mriedem: but inside the lock | |
| 16:52:22 | mriedem | i was just going to use self._default_ctxt_mgr | |
| 16:52:55 | dansmith | not sure what you mean | |
| 16:53:26 | dansmith | mriedem: before this, you need to yield if cell_mapping is None and then return https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L402-L402 | |
| 16:53:26 | mriedem | sec | |
| 16:54:35 | mriedem | here it comes | |
| 16:55:00 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Decrement quota when deleting an instance in cell0 https://review.openstack.org/443395 | |
| 16:55:02 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add regression test for bug 1670627 https://review.openstack.org/443745 | |
| 16:55:02 | openstack | bug 1670627 in OpenStack Compute (nova) "quota is always in-use after delete the ERROR instances " [Critical,In progress] https://launchpad.net/bugs/1670627 - Assigned to Dan Smith (danms) | |
| 16:55:02 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Temporarily untarget context when deleting from cell0 https://review.openstack.org/443403 | |
| 16:56:25 | dansmith | mriedem: I'd prefer you didn't do that, | |
| 16:56:52 | mriedem | i'm fine changing it | |
| 16:56:58 | dansmith | mriedem: not only because it ends up mutating the context unlike the real live scenario, but it also is database-specific, so when my mq one rolls around, it will target the mq but not the db | |
| 16:58:41 | mriedem | hmm, so that makes the test fail | |
| 17:01:50 | mriedem | dansmith: wouldn't the default context be pointing at the main cell db? | |
| 17:02:01 | mriedem | which is what we want when we create the reservation | |
| 17:02:22 | dansmith | mriedem: it points at the one that was default=true, but in real life you'll end up taking the global one instead of having your cell targeted at the other one | |
| 17:02:30 | dansmith | mriedem: which means it depends on their config, and won't look like it does in real life | |
| 17:02:55 | dansmith | mriedem: so someone will come along and want to assert that db_connection=None there because that is how target_cell works, but it will be different when using the fixture | |
| 17:04:13 | arne_r | Calling attention to https://review.openstack.org/#/c/436868/ which, if passed, would make our CI folks really happy | |
| 17:04:37 | mriedem | dansmith: ok, in the nova/cinder meeting now, you want to tinker with that locally to change it? | |
| 17:04:52 | mriedem | because i'm a bit lost in that fixture | |
| 17:04:55 | dansmith | mriedem: sure | |
| 17:05:26 | dansmith | it's complicated | |
| 17:05:40 | dansmith | mriedem: you said the test failed.. which one did you mean? your regression test should fail yes? | |
| 17:06:16 | mriedem | https://review.openstack.org/443403 is passing | |
| 17:06:23 | mriedem | when i changed to what you wanted in the fixture, it failed | |
| 17:06:42 | mriedem | so to do what you want in the fixture, i need you to take a look at why it's failing | |
| 17:06:43 | dansmith | mriedem: but which test failed, the regression test? | |
| 17:06:46 | mriedem | yes | |
| 17:06:51 | mriedem | i'm just running that one | |
| 17:07:06 | dansmith | oh I see you changed it there, gotcha | |
| 17:07:31 | dansmith | waiting for a py27 run to finish and then I'll pull it down | |
| 17:08:46 | mriedem | py27 for something else right? | |
| 17:09:54 | dansmith | yes | |
| 17:11:20 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: fix missing instance.delete notification https://review.openstack.org/437222 | |
| 17:12:47 | mriedem | ok, once i'm done with this meeting i'll fix up the unit tests on the middle patch | |
| 17:18:37 | dansmith | mriedem: actually, I changed my mind, this is okay | |
| 17:19:03 | dansmith | I'm not sure it's as obvious, but it's less fiddling, and the context still looks like it should because we call real_target_cell anyway | |
| 17:19:13 | dansmith | mriedem: for completeness let me show you what I was thinking: | |
| 17:19:39 | dansmith | mriedem: http://pastebin.com/raw/isCJMvhG | |
| 17:20:06 | dansmith | mriedem: that works, but it may be more confusing for other people | |
| 17:20:11 | dansmith | if you agree, then just stick with what you've got | |
| 17:20:52 | mriedem | "controllering"? | |
| 17:20:54 | dansmith | http://pastebin.com/raw/pNv5smz8 | |
| 17:20:55 | mriedem | i disagree with that | |
| 17:20:55 | dansmith | is what it looks like applied | |
| 17:21:05 | dansmith | hah I just saw that | |
| 17:21:12 | dansmith | unintentional, but sounds like me | |
| 17:22:14 | mriedem | so, your change, it's basically just a pass through to the real unwrapped thing, right? | |
| 17:22:29 | dansmith | skipping the global db state switch, yeah | |
| 17:22:42 | mriedem | ok i think it makes sense to me, enough ayway | |
| 17:22:43 | mriedem | *anyway | |
| 17:22:55 | dansmith | yours does the switch, but .. to the main one | |
| 17:23:54 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transform missing delete notifications https://review.openstack.org/410297 | |
| 17:23:55 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: use context mgr in instance.delete https://review.openstack.org/443764 | |
| 17:24:17 | mriedem | ok i'll just take your change and update my local copy | |
| 17:24:36 | gibi | mriedem, dansmith: I splitted the instance.delete bugfix patch as we discussed https://review.openstack.org/#/c/437222 | |
| 17:24:55 | dansmith | mriedem: totally your call | |
| 17:25:24 | gibi | mriedem, dansmith: but I have to run now. thanks for the reviews! | |
| 17:25:25 | dansmith | gibi: I was going to suggest that first patch not adding/using the contextmanager | |
| 17:25:39 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: introduce basic framework for security proxying https://review.openstack.org/345396 | |
| 17:25:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: introduce framework for RFB authentication https://review.openstack.org/345397 | |
| 17:25:40 | dansmith | gibi: but if it's not going to cause conflicts, it's not as much of a problem I guess | |
| 17:25:40 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: introduce the VeNCrypt RFB authentication scheme https://review.openstack.org/345398 | |
| 17:25:41 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: provide an RFB security proxy implementation https://review.openstack.org/345399 | |
| 17:26:11 | gibi | dansmith: it didn't caused conflict a couple of days ago when I cherry-picked the patch to stable/ocata | |
| 17:26:55 | dansmith | gibi: not now for sure, I just mean in terms of backporting less code. the context manager add is more code, but it's contained | |
| 17:26:56 | dansmith | gibi: if mriedem is cool with it then it's fine | |
| 17:26:56 | gibi | dansmith: here is the cherry-pick https://review.openstack.org/#/c/441171/ I will update that as well | |
| 17:27:08 | gibi | dansmith: ahh I see | |
| 17:27:23 | gibi | dansmith: I agree that the ctx manager is quite self contained solution | |
| 17:27:43 | dansmith | gibi: anyway, I'll look at this in a few, thanks! | |
| 17:27:52 | gibi | dansmith: thanks a lot! | |
| 17:28:05 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Fix formatting of network options https://review.openstack.org/443086 | |
| 17:28:58 | mriedem | catch him! | |
| 17:29:47 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Final cleanups in conf/network https://review.openstack.org/431023 | |
| 17:30:16 | openstackgerrit | John Garbutt proposed openstack/nova-specs master: Add spec to use cinder's new attachment API https://review.openstack.org/373203 | |
| 17:34:19 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Temporarily untarget context when deleting from cell0 https://review.openstack.org/443403 | |
| 17:41:51 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/nova-specs master: Enable SR-IOV NIC offload feature discovery https://review.openstack.org/435954 | |
| 17:43:30 | openstackgerrit | Sarafraj Singh proposed openstack/nova-specs master: Spec to add per instance timeout for LM https://review.openstack.org/442809 | |
| 17:44:12 | openstackgerrit | Feodor Tersin proposed openstack/nova master: Give names to volumes during block device mapping https://review.openstack.org/213433 | |
| 17:47:37 | openstackgerrit | Rafael Folco proposed openstack/nova master: DELETE all inventory for a resource provider https://review.openstack.org/416669 | |
| 17:50:58 | dansmith | mriedem: I'm going to pull down gibi's patch and fix a test comment of mine | |
| 17:51:26 | dansmith | mriedem: are you cool with the heavier context manager approach for the backport or do you want to do the simplest thing, which is just call the normal methods in the places where we're not? | |
| 17:51:56 | dansmith | I think his approach here ends up getting us a begin/end notification that we wouldn't get otherwise | |
| 17:52:03 | dansmith | which is probably good, it's just .. more code | |
| 17:59:04 | mriedem | if it's not making the backport a mess i'm fine with it | |
| 17:59:09 | mriedem | i haven't dug in in detail | |
| 18:03:11 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix missing instance.delete notification https://review.openstack.org/437222 | |
| 18:03:51 | dansmith | I also sneakily capitalized the first letter of the commit message because .. I hate that. | |
| 18:05:13 | edleafe | rfolco: A could of nits on https://review.openstack.org/416669, but one serious issue | |
| 18:07:00 | rfolco | edleafe, ack | |
| 18:10:31 | mriedem | dansmith: but not period at the end? | |
| 18:10:34 | mriedem | *no | |
| 18:10:37 | dansmith | no I hate theperiod | |
| 18:10:45 | mriedem | I like pizza. I also like cookies. | |
| 18:10:55 | dansmith | heh | |
| 18:14:47 | openstackgerrit | Rafael Folco proposed openstack/nova master: DELETE all inventory for a resource provider https://review.openstack.org/416669 | |
| 18:36:53 | mdbooth | ... when you realise you created dicts called host_cell_mapping_mapping and cell_mapping_host_mapping | |