| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-30 | |||
| 18:16:38 | openstackgerrit | Eric Fried proposed openstack/nova master: Proper error handling by _ensure_resource_provider https://review.openstack.org/524263 | |
| 18:16:50 | efried | cdent Not sure if I addressed your concerns here ^ | |
| 18:17:23 | efried | cdent I wouldn't think it necessary to say, "Exceptions raised from here may be caught, logged, or ignored by callers as appropriate" | |
| 18:19:11 | efried | cdent Really just brings things more in line with other methods in SRC, like _update_inventory_attempt which may raise InventoryInUse | |
| 18:19:40 | melwitt | mriedem: *reading the commit message* so your test doesn't always fail though? trying to confirm if the test passing means the issue in my patch was addressed by the latest changes I tried | |
| 18:20:04 | cdent | efried: that wasn’t quite what I meant. You’re obliged, as someone who has introduced a new situation, where exceptions are now raising that weren’t before, to make sure they don’t break other stuff. That can be as simple as identifying the loop where all Exceptions are handled in the scheduler or the resource trackers servers. | |
| 18:20:47 | efried | cdent OIC. Not enough to pass the CI? | |
| 18:21:13 | cdent | We’ve established over and over that CI isn’t covering lots of stuff, especially in the resource tracker | |
| 18:21:54 | cdent | It’s very likely, however, that this _is_ covered, by the kinds of loops that I mention above, but if you don’t say that in your commit message, then reviewers have doubt and we want to minimize that doubt | |
| 18:22:08 | efried | cdent I was being mostly ironical. I'll dig into it. Thanks. | |
| 18:26:31 | efried | cdent AFAICT, it all percolates up to this catch-all, which... ignores it https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L7105 | |
| 18:27:43 | cdent | i assumed it would be something like that, and logging and ignoring is what I would expect: it’s a long running process that’s not allowed to die | |
| 18:28:13 | cdent | but if for some reason that was some subclass of Exception, then there would be a chance your new exception would fall through and boom | |
| 18:30:53 | openstackgerrit | Eric Fried proposed openstack/nova master: Proper error handling by _ensure_resource_provider https://review.openstack.org/524263 | |
| 18:30:59 | efried | cdent Added more words ^ | |
| 18:39:09 | mriedem | melwitt: it was failing | |
| 18:45:44 | openstackgerrit | Eric Fried proposed openstack/nova master: Remove workaround for bug #1709118 https://review.openstack.org/524316 | |
| 18:45:45 | openstack | bug 1709118 in OpenStack Compute (nova) "_ContextAuthPlugin needs a refresh" [Low,Confirmed] https://launchpad.net/bugs/1709118 | |
| 18:54:35 | mriedem | dansmith: comments in https://review.openstack.org/#/c/524253/2 | |
| 18:54:43 | mriedem | just nits, tests are passing, looks good otherwise | |
| 18:57:18 | melwitt | mriedem: hm, so that points to it being related to the periodic task bug dansmith found in my patch. but the initial write of the compute node record doesn't happen via periodic task, it happens through the pre_start_hook() call in Service. so I'm not sure why fixing the periodic task helps | |
| 18:58:30 | dansmith | mriedem: see my reply real quick | |
| 18:58:53 | dansmith | melwitt: a periodic would create another record in cell0 if it runs | |
| 18:59:04 | dansmith | that was my point | |
| 18:59:22 | melwitt | dansmith: right ... trying to think how that could mess up mriedem's test though, where it got ComputeHostNotFound | |
| 18:59:41 | melwitt | like wouldn't it find the original record that was written via pre_start_hook() | |
| 19:00:15 | dansmith | melwitt: ack, okay, I haven't looked at his thing so I dunno | |
| 19:00:20 | dansmith | just commenting from the sidelines | |
| 19:00:56 | melwitt | ack :) | |
| 19:01:29 | mriedem | dansmith: done | |
| 19:02:08 | melwitt | fixing the periodic task thing (by storing it on Service) made the test pass but why O.o | |
| 19:02:30 | dansmith | mriedem: cool, see the split above it? | |
| 19:03:58 | mriedem | yeah, just haven't reviewed it yet | |
| 19:05:24 | dansmith | gahd dammit | |
| 19:05:37 | dansmith | making changes to a file underneath a rename is annoying :) | |
| 19:08:04 | mriedem | cdent: easy cleanup if you want to respin this https://review.openstack.org/#/c/484667/ | |
| 19:08:06 | mriedem | then i'd fast approve | |
| 19:08:52 | cdent | cool, thanks, will get it in a couple minutes, rado’s usually gone by now | |
| 19:09:21 | openstackgerrit | Dan Smith proposed openstack/nova master: Genericify the instance_list stuff https://review.openstack.org/524253 | |
| 19:09:22 | openstackgerrit | Dan Smith proposed openstack/nova master: Split instance_list into instance and multi_cell https://review.openstack.org/524302 | |
| 19:10:02 | openstackgerrit | Merged openstack/nova-specs master: Create specs directory for Rocky https://review.openstack.org/514101 | |
| 19:10:31 | cfriesen | melwitt: do you know if anyone has considered adding a "list all tenants with modified quotas" and "list all users in a tenant with modified quotas" APIs? As it is there's no way to get that information without polling *all* the users/tenants. | |
| 19:11:17 | melwitt | cfriesen: I'm not aware of anyone bringing that up before, no | |
| 19:12:11 | cfriesen | melwitt: I'm looking at kingbird (which replicates quotas in a multi-cloud environment) and they literally have to poll everything. | |
| 19:12:38 | cfriesen | also, it looks like the quota usage (and the "limits" API) doesn't report keypair usage, so they have to manually count them per user. | |
| 19:14:57 | melwitt | I see. ironically when I implemented counting quotas I had to set in_use = 0 for keypairs to keep the same behavior. I was thinking we might have to microversion to change that or otherwise signal it | |
| 19:17:01 | melwitt | so yeah, there are probably many improvements that could happen with the APIs. one thing to consider is that the keystone team is looking to add quota limits to keystone. which would make things separated where you'd have to query keystone for limits and nova for usage | |
| 19:22:15 | openstackgerrit | Jay Pipes proposed openstack/nova master: Use oslo_db Session in resource_provider.py https://review.openstack.org/523192 | |
| 19:23:27 | jaypipes | efried: are you currently working on https://review.openstack.org/#/c/517119/? | |
| 19:24:10 | openstackgerrit | Chris Dent proposed openstack/nova master: VMware: Handle concurrent registrations of the VC extension https://review.openstack.org/484667 | |
| 19:25:30 | cdent | mriedem: ^ thanks | |
| 19:27:01 | cfriesen | melwitt: do you know what the timeline is on the keystone quotas? If it's going to be a while I might have to propose an API change. | |
| 19:27:23 | mriedem | cdent: thanks - i was coding that in the comment out of memory, does it actualy pass the test? | |
| 19:27:31 | cdent | yes | |
| 19:27:35 | mriedem | cool | |
| 19:27:46 | efried | jaypipes No | |
| 19:28:03 | cdent | and I went to look at the excutils code just to make sure it made sense (because it was new to me) and your example matched the docstring’s example | |
| 19:28:10 | jaypipes | efried: k, I'm going to pull and work on that then | |
| 19:28:17 | melwitt | cfriesen: you'll want to ask lbragstad about it as he's in the know about keystone timelines | |
| 19:28:17 | efried | jaypipes Cool beans. | |
| 19:28:45 | cdent | efried, jaypipes : is there a new stable review-entry point for n-r-p? | |
| 19:29:07 | jaypipes | cdent: unfortunately not. | |
| 19:29:14 | efried | cdent I thought it was here: https://review.openstack.org/#/c/377215/ | |
| 19:29:47 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: allow filter providers in tree https://review.openstack.org/377215 | |
| 19:29:48 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: adds REST API for nested providers https://review.openstack.org/384807 | |
| 19:29:48 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: update client to set parent provider https://review.openstack.org/385693 | |
| 19:29:48 | efried | cdent The bottom three patches | |
| 19:29:54 | lbragstad | cfriesen: we're in the process of firming up the specification for unified limits https://review.openstack.org/#/c/455709/ (wxy_ is actively responding to comments and making updates) | |
| 19:29:59 | efried | Oh, just updated | |
| 19:30:14 | jaypipes | cdent: I just addressed that microversion.py rebase flub, nothing more. | |
| 19:30:22 | cdent | roger | |
| 19:30:29 | lbragstad | cfriesen: wxy_ also has an early implementation proposed for review | |
| 19:30:43 | lbragstad | cfriesen: https://review.openstack.org/#/q/status:open+project:openstack/keystone+branch:master+topic:bp/unified-limits | |
| 19:31:02 | cdent | I think my best plan is perhaps to be done with openstack for today | |
| 19:31:30 | cdent | have fun everyone | |
| 19:32:31 | lbragstad | cfriesen: specification freeze for keystone is next week - so we're trying to get the details ironed out this week and next (all feedback welcome!) | |
| 19:33:36 | openstackgerrit | Jay Pipes proposed openstack/nova master: handle traits with sharing providers https://review.openstack.org/517119 | |
| 19:33:36 | openstackgerrit | Jay Pipes proposed openstack/nova master: Test alloc_cands with non overlapping sharing RPs https://review.openstack.org/519380 | |
| 19:33:37 | openstackgerrit | Jay Pipes proposed openstack/nova master: Test alloc_cands with one RP shared between two RPs https://review.openstack.org/519617 | |
| 19:33:37 | openstackgerrit | Jay Pipes proposed openstack/nova master: Test allocation candidates: multiple aggregates https://review.openstack.org/518633 | |
| 19:33:38 | openstackgerrit | Jay Pipes proposed openstack/nova master: Test helper: validate provider summaries https://review.openstack.org/518982 | |
| 19:33:38 | openstackgerrit | Jay Pipes proposed openstack/nova master: Fix accumulated nits in refactor series https://review.openstack.org/521189 | |
| 19:33:53 | jaypipes | efried: rebased and fixed conflicts up to ^ | |
| 19:34:44 | jaypipes | efried: I stopped there because I don't know which of alex and tetsuro's add func tests patches are duplicated and which are real. | |
| 19:35:00 | efried | jaypipes ack | |
| 19:56:55 | melwitt | mriedem: turns out your test just always passes locally for me (on top of my celldatabases patch) even without the periodic task fix. so something must be afoot if you're getting a different result locally than me | |
| 19:58:47 | mriedem | melwitt: at some point i rebased out your change since it wasn't fixing stuff, that's when i started writing that workaround in start_service | |
| 19:59:13 | melwitt | oh, okay | |
| 20:01:19 | melwitt | if you still had my change when you fixed the wrong assert you mentioned earlier and it didn't pass, then that's really unexpected | |
| 20:04:30 | mriedem | unrelated, but if you use gerrit-dash-creator for your nova review dashboard, it's been fixed for zuul https://github.com/openstack/gerrit-dash-creator/blob/master/dashboards/nova.dash | |
| 20:04:42 | mriedem | i was wondering why my dashboard was showing me changes with a -2 as ready for final +2 | |
| 20:05:09 | openstackgerrit | Jackie Truong proposed openstack/python-novaclient master: Microversion 2.57 - Add trusted_image_certificates https://review.openstack.org/500396 | |
| 20:05:37 | melwitt | but I need to re-create my dashboard right? my old dashboard doesn't work anymore | |
| 20:05:56 | mriedem | correct | |
| 20:06:06 | mriedem | you have to git clone gerrit-dash-creator | |
| 20:06:15 | mriedem | and then: tox -e venv -- gerrit-dash-creator dashboards/nova.dash | |
| 20:06:15 | melwitt | okay, thanks | |
| 20:06:29 | mriedem | i put the url in a shortener and then put that in my gerrit settings | |
| 20:07:33 | mriedem | efried: seen this https://review.openstack.org/#/c/521764/ ? | |
| 20:07:36 | melwitt | sweet. good tips | |