| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-19 | |||
| 15:58:20 | fried_rice | cfriesen Yeah, I see _update_port_binding_for_instance takes a migration kwarg | |
| 15:58:36 | leakypipes | cfriesen: +Wallaby'd | |
| 15:58:49 | cfriesen | leakypipes: awesome, thanks | |
| 15:59:08 | fried_rice | cfriesen Left a passive-aggressive +0 with a nit. | |
| 15:59:49 | cfriesen | lol | |
| 16:00:28 | leakypipes | jackiet: how about "fullmetaljackie"? ;) | |
| 16:00:47 | leakypipes | jackiet: though that might be too long for irc. | |
| 16:01:52 | fried_rice | ++! | |
| 16:01:53 | fullmetaljackiet | leakypipes: niceeee I like it | |
| 16:01:57 | leakypipes | heh | |
| 16:02:00 | leakypipes | :) | |
| 16:02:12 | leakypipes | fullmetaljackiet: glad to be of service. | |
| 16:02:13 | cfriesen | leakypipes: want me to quick make the change suggested by fried_rice? | |
| 16:02:59 | leakypipes | cfriesen: meh? the way the gate is I'd prefer to just leave it for now. | |
| 16:03:08 | cfriesen | works for me | |
| 16:03:15 | cfriesen | figured I'd offer | |
| 16:03:16 | leakypipes | fried_rice is right, of course. | |
| 16:03:22 | leakypipes | but it's a nit | |
| 16:03:49 | fried_rice | Not even worth a followon. | |
| 16:04:56 | leakypipes | fried_rice: so, your comment on this one unit test "_get_provider_aggregates and _traits were called thrice" just makes me think of the monty python holy hand grenade scene. | |
| 16:05:13 | fried_rice | leakypipes Five is RIGHT OUT. | |
| 16:05:19 | leakypipes | "The number shall be THREE!" | |
| 16:05:28 | leakypipes | :) | |
| 16:05:33 | fried_rice | Then lobbest thou the holy hand grenade... | |
| 16:05:37 | fried_rice | okay okay. | |
| 16:05:40 | leakypipes | hehe | |
| 16:05:54 | fried_rice | Back to Airplane! quotes. | |
| 16:06:20 | openstackgerrit | Matthew Edmonds proposed openstack/nova master: Fix bad autospec usage https://review.openstack.org/535806 | |
| 16:07:17 | edmondsw | claudiub I would prefer ^ to the oslotest fixture | |
| 16:07:33 | edmondsw | but it's at least a first step even if you want to continue to push use of that fixture | |
| 16:07:42 | edmondsw | (which could be done subsequently) | |
| 16:08:58 | bauzas | ildikov: I don't see any change in the series that does things at the driver level with https://review.openstack.org/#/c/531244/10/nova/compute/manager.py@5190 | |
| 16:09:16 | bauzas | ildikov: I mean, you and matt made the RPC changes for asking for multiattach, sure | |
| 16:09:31 | bauzas | but then, how do we pass that down to libvirt? I'm confused | |
| 16:10:03 | bauzas | oh f***, nevermind | |
| 16:10:05 | ildikov | bauzas: it's in the capabilities | |
| 16:10:26 | ildikov | bauzas: I guess you found it :) | |
| 16:10:30 | bauzas | ildikov: yeah, so basically the RPC change is just for asking whether the compute driver supports it | |
| 16:10:37 | ildikov | bauzas: yes | |
| 16:10:40 | bauzas | if not, raise | |
| 16:10:45 | ildikov | exactly | |
| 16:10:51 | bauzas | if yes, let's the volume say whether it's multiattached | |
| 16:10:54 | bauzas | gotcha | |
| 16:10:59 | claudiub | edmondsw: sure, that works too, but i've had nightmares with mock.create_autospec | |
| 16:11:18 | bauzas | ildikov: sorry, I'm in general used to see things happening at the manager level once you pass a new attribute | |
| 16:11:27 | bauzas | a new *RPC* attribute | |
| 16:11:27 | ildikov | bauzas: yeah, many pieces to put together | |
| 16:11:34 | claudiub | I've had run times increase exponentially because of mock.create_autospec | |
| 16:11:43 | bauzas | ildikov: yeah, https://review.openstack.org/#/c/532928/4/nova/compute/manager.py does that | |
| 16:12:26 | edmondsw | claudiub hmm | |
| 16:12:33 | ildikov | bauzas: we tried to look into failing as quick as possible if either bit is not there | |
| 16:13:06 | claudiub | at one time, i've had an execution time of 4000 seconds for ~300 unit tests. :) | |
| 16:13:18 | claudiub | aprox. | |
| 16:13:21 | ildikov | bauzas: as we don't have all the info at the API we needed some tricks :) | |
| 16:14:40 | bauzas | ildikov: I'll +W it, but I think you should add a NOTE in the manager explaining why you don't really care doing things after the RPC call | |
| 16:15:08 | bauzas | ildikov: just because it took me some time to understand that, some other people wanting to contribute could be confused by that new attribute we don't really use | |
| 16:15:15 | bauzas | it's just a flag | |
| 16:15:53 | ildikov | bauzas: sure, I guess you added a comment already, so I will do it in a follow up patch | |
| 16:16:00 | ildikov | bauzas: thank you | |
| 16:16:04 | bauzas | ildikov: yup, no rush | |
| 16:16:12 | bauzas | ildikov: that can totally be a follow-up | |
| 16:16:23 | ildikov | bauzas: cool :) | |
| 16:16:26 | bauzas | but I'd be happy if we could keep sort of the logic in the code | |
| 16:16:41 | bauzas | matt did a good job doing that in the conductor | |
| 16:16:58 | ildikov | yeah, it always starts simple and then we give it more and more thoughts... | |
| 16:17:19 | bauzas | ildikov: for example https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L896-L903 | |
| 16:17:26 | ildikov | the tricks are all on him here too and I mean it the best way possible | |
| 16:17:45 | bauzas | and https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L913-L921 | |
| 16:18:00 | bauzas | ildikov: sure, I don't deny it | |
| 16:18:02 | ildikov | nice, good examples | |
| 16:18:13 | bauzas | ildikov: I'm just saying, we somehow need to remember what we did | |
| 16:18:28 | ildikov | bauzas: I completely agree | |
| 16:18:44 | bauzas | anyway, I'm calling it a day | |
| 16:19:02 | bauzas | the API change is hairy and not my expertise, so it'll require a bit more thoughts | |
| 16:19:11 | bauzas | not for a friday thing | |
| 16:19:15 | bauzas | ++ folks | |
| 16:19:34 | ildikov | bauzas: makes sense, thanks for looking into the series | |
| 16:20:24 | ildikov | bauzas: have a great weekend! | |
| 16:21:33 | lyarwood | mdbooth: http://logs.openstack.org/58/523958/16/check/openstack-tox-py35/db3ef07/testr_results.html.gz - if you have time, any thoughts on these failures? hasattr is failing on o.vo objects on py35, works on py27. | |
| 16:22:41 | mdbooth | lyarwood: Yeah, will look now. | |
| 16:27:04 | mdbooth | lyarwood: I'm hacking on it right now. Ping me if you fix it before I do. | |
| 16:27:15 | lyarwood | mdbooth: the race is on | |
| 16:27:21 | mdbooth | Hehe | |
| 16:27:30 | mdbooth | If you've got other stuff to do, feel free :) | |
| 16:27:31 | lyarwood | mdbooth: but given I pinged as I was stuck I think you might win | |
| 16:42:49 | edmondsw | claudiub I ran the UTs for test_compute (which take the longest of the files changed) for both master and my proposal with create_autospec. No significant perf impact | |
| 16:43:09 | edmondsw | not saying there couldn't be an impact somewhere else, but not substantial in that file anyway | |
| 16:44:27 | edmondsw | ran 1934 tests in 336s without and 360s with create_autospec (on my slow laptop env) | |
| 16:53:29 | mdbooth | lyarwood: I've found the root cause | |
| 16:54:37 | mdbooth | lyarwood: It's a difference in the implementation of hasattr between python2 and python3: http://paste.openstack.org/show/647153/ | |
| 16:55:37 | mdbooth | What the hell do you do about that? is question 2 | |
| 16:57:59 | lyarwood | mdbooth: cool thanks, I was sure I hit something like this before but can't seem to find anything | |
| 16:58:33 | lyarwood | mdbooth: https://review.openstack.org/#/c/342111/13 for example, actually introduced hasattr to get py34 passing at the time | |
| 17:00:43 | mdbooth | It's a bug in OVO imho | |
| 17:01:05 | mdbooth | It seems python expects you to raise AttributeError in a __gettattr__ | |
| 17:01:39 | mdbooth | But obj_load_attr is raising NotImplementedError | |
| 17:02:07 | lyarwood | mdbooth: right and 2.7 catches any exception | |
| 17:02:09 | lyarwood | mdbooth: https://docs.python.org/2/library/functions.html?highlight=hasattr#hasattr | |
| 17:02:26 | lyarwood | 99% sure I've hit this before and there was a workaround | |
| 17:02:50 | mdbooth | The Nova fix would be to override obj_load_attr() and raise AttributeError instead of NotImplementedError | |