| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-19 | |||
| 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 | |
| 17:03:19 | mdbooth | However, we should also submit a patch to ovo | |
| 17:03:23 | openstackgerrit | Merged openstack/os-traits master: Add CPU features for AArch64 https://review.openstack.org/523665 | |
| 17:03:32 | mdbooth | https://docs.python.org/3/library/functions.html#hasattr | |
| 17:06:17 | mdbooth | lyarwood: Hmm, it's messier than that because we also call it in other contexts. Some subclasses raise ObjectActionError | |
| 17:06:42 | mdbooth | lyarwood: So I think this calls for ocean-boiling-avoidance | |
| 17:07:09 | mdbooth | Although it is a bug in OVO | |
| 17:07:36 | claudiub | edmondsw: sounds ok in that case, but in other cases, it can really drive up the execution time. I've tried it now for nova.tests.unit.virt.hyperv.test_vmops, only using mock.create_autospec for a few instance attributes, and the execution time jumped from 3.8 sec / 121 tests to 7.5 sec / 121 tests. Again, not everything was autospec'd :) | |
| 17:08:02 | lyarwood | mdbooth: using obj_load_attr you mean? | |
| 17:08:49 | edmondsw | claudiub is that comparing to master or to the oslotests fixture? | |
| 17:08:49 | mdbooth | lyarwood: ovo should handle the lack of contract for obj_load_attr | |
| 17:09:12 | mdbooth | If we're going to port OpenStack to a new language, there are better options than python3 | |
| 17:09:25 | claudiub | mdbooth: mock.create_autospec autospecs literally everything in the given object / class, which is not really necessary for every single test | |
| 17:09:47 | claudiub | while mock.Mock(autospec=) only autospecs something when accessed. | |
| 17:09:59 | edmondsw | claudiub and I'm not sure that's a big deal... you can look at as twice as long (bad) or as 4 extra seconds (insignificant). Would need to see impact on a larger scale | |
| 17:10:21 | mdbooth | claudiub: nice | |
| 17:11:03 | edmondsw | claudiub that does sound nice though | |
| 17:11:39 | claudiub | edmondsw: 3.929s per 121 tests | |
| 17:13:22 | claudiub | ok, so, 3.8 sec with no change. 3.929 with mock.Mock(autospec) (oslotest), and 7.5 sec with mock.create_autospec | |
| 17:15:03 | mdbooth | lyarwood: The workaround is to use getattr instead, because that has consistent behaviour | |
| 17:15:11 | mdbooth | i.e. it raises an exception | |
| 17:16:39 | mdbooth | lyarwood: Assuming you've got local changes, I'll leave ^^^ in a review comment | |
| 17:16:52 | lyarwood | mdbooth: yeah thanks | |
| 17:18:34 | mdbooth | https://bugs.launchpad.net/oslo.versionedobjects/+bug/1552927 | |
| 17:18:35 | openstack | Launchpad bug 1552927 in oslo.versionedobjects "hasattr() function behavior differences between Py27 and Py34" [Undecided,New] | |
| 17:18:43 | mdbooth | lyarwood: Already been reported ^^^^ | |
| 17:19:34 | mdbooth | obj.attr_is_set()... | |
| 17:21:16 | lyarwood | mdbooth: ah! | |
| 17:21:42 | mdbooth | Just testing that | |
| 17:22:34 | mdbooth | lyarwood: Works, at least for that test | |
| 17:22:48 | lyarwood | mdbooth: yeah works for me | |
| 17:23:48 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: QEMU native LUKS decryption for encrypted volumes https://review.openstack.org/523958 | |
| 17:35:28 | claudiub | mdbooth: gonna send you a link soon. already 60 seconds passed while trying to execute unittests which shouldn't last more than 10 seconds. :) | |
| 17:42:40 | claudiub | mdbooth: https://github.com/bclau/nova/tree/autospec-issue | |
| 17:43:22 | mdbooth | lyarwood: I'd have left you a +1 if Jenkins had voted already. I'd appreciate if you consider the readability of the new test, though. | |
| 17:43:53 | mdbooth | My eyeballs aren't good at computing diffs. | |
| 17:43:54 | claudiub | execution time: vanilla: ~10 seconds. oslotest mock.MagicMock(autospec): ~14.5 sec. mock.create_autospec: I dunno, a few hundred seconds, still running. | |
| 17:44:54 | mdbooth | claudiub: That's great info, and definitely something I hadn't appreciated. | |
| 17:45:11 | mdbooth | I think I've used create_autospec in a few places in the past. | |
| 17:45:53 | claudiub | it's definetely great, but the fact that it autospecs literally everything can be problematic. | |
| 17:46:19 | claudiub | and I have sent pull requests to python 3 and the mock library, but no response for at least half a year | |
| 17:46:35 | claudiub | and there hasn't been any updates to the mock library since more than 12-13 months | |
| 17:47:03 | claudiub | Ran 364 tests in 434.626s | |
| 17:53:05 | openstackgerrit | David Rabel proposed openstack/nova stable/pike: Fix format in live-migration-usage.rst https://review.openstack.org/535871 | |
| 18:00:27 | mdbooth | Incoming | |
| 18:01:03 | openstackgerrit | Matthew Booth proposed openstack/nova master: Remove redundant swap_volume tests https://review.openstack.org/531179 | |
| 18:01:04 | openstackgerrit | Matthew Booth proposed openstack/nova master: Give volume DriverBlockDevice classes a common prefix https://review.openstack.org/526346 | |
| 18:01:04 | openstackgerrit | Matthew Booth proposed openstack/nova master: Add DriverLocalImageBlockDevice https://review.openstack.org/526347 | |
| 18:01:05 | openstackgerrit | Matthew Booth proposed openstack/nova master: Expose driver_block_device fields as attributes https://review.openstack.org/528362 | |
| 18:01:05 | openstackgerrit | Matthew Booth proposed openstack/nova master: Add local_root to block_device_info https://review.openstack.org/529029 | |
| 18:01:06 | openstackgerrit | Matthew Booth proposed openstack/nova master: Pass DriverBlockDevice to driver.attach_volume https://review.openstack.org/528363 | |
| 18:01:06 | openstackgerrit | Matthew Booth proposed openstack/nova master: Use real block_device_info data in libvirt tests https://review.openstack.org/527916 | |
| 18:01:07 | openstackgerrit | Matthew Booth proposed openstack/nova master: Fix libvirt volume tests passing invalid disk_info https://review.openstack.org/529328 | |
| 18:01:07 | openstackgerrit | Matthew Booth proposed openstack/nova master: Pass disk_info dict to libvirt_info https://review.openstack.org/529329 | |
| 18:01:08 | openstackgerrit | Matthew Booth proposed openstack/nova master: Expose volume host type and path independent of libvirt config https://review.openstack.org/530786 | |
| 18:01:09 | openstackgerrit | Matthew Booth proposed openstack/nova master: Don't generate fake disk_info in swap_volume https://review.openstack.org/530787 | |
| 18:01:09 | openstackgerrit | Matthew Booth proposed openstack/nova master: Local disk serial numbers for the libvirt driver https://review.openstack.org/529380 | |
| 18:15:36 | lyarwood | mdbooth: ack, I might rewrite both tests using the conf objects and format_dom calls | |
| 18:38:02 | openstackgerrit | Jim Rollenhagen proposed openstack/nova master: Ironic: negotiate microversion to allow downgrade to Pike https://review.openstack.org/535786 | |
| 19:13:34 | cfriesen | found an interesting glance/nova interworking issue. if doing a snapshot, and cinder/glance are both on ceph, nova tries to shortcut the snapshot then set the location in glance. But by default glance won't let you set the location of an image, so this fails. | |
| 19:16:09 | cfriesen | correction, not cinder, but when instance backend is ceph. | |
| 19:57:39 | cburgess | cfriesen Sounds like nova is doing the right thing but glance just isn't configured properly. Sounds like maybe a release note or something is needed? | |
| 19:57:54 | cburgess | cfriesen I suspect cinder will run into the same issue. | |
| 20:22:17 | fried_rolls | you gotta be effin kidding me. The bottom three patches, all +W'd, got zuul -1 with (three *different*) spurious failures. The fourth patch - the first one with a -1 - passed. | |
| 20:24:05 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/535919 | |
| 20:25:32 | ildikov | fried_rice: mine got a time_out error on the gate, sooo close, but no... | |
| 20:26:46 | fried_rice | Boo. I feel your pain. | |
| 20:43:30 | fried_rice | figleaf regarding https://review.openstack.org/#/c/526540/17/nova/scheduler/client/report.py@390 -- the only possible reason I can see for it having been split out before was LOG.warning vs .error. | |
| 20:43:59 | fried_rice | figleaf But given that the error text is (at least) as descriptive as what was there before, are you cool with it? | |
| 20:55:35 | openstackgerrit | Ihar Hrachyshka proposed openstack/nova master: DNM testing whether lib/neutron switch breaks this repo https://review.openstack.org/535947 | |
| 21:02:27 | cfriesen | cburgess: I've been diving down into the rathole of the nova/glance interaction. turns out glance's policy stuff is all mucked up, so that enabling nova's snapshot in this case requires glance to be configured insecurely. It might be possible to use service tokens to fix it, but nova doesn't send them by default. | |
| 21:02:39 | cfriesen | speaking of which...anyone know why send_service_user_token is False by default? | |
| 21:03:19 | cfriesen | cburgess: the glance folks suggested a workaround of a separate glance node that is configured less secure, which is only accessible by other openstack components. kind of icky if you ask me. | |
| 21:08:02 | figleaf | fried_rice: It just seemed like a bigger behavior change than it set out to be | |
| 21:08:13 | figleaf | fried_rice: it's not a showstopper by any stretch | |
| 21:08:20 | fried_rice | coolcool. | |
| 21:09:58 | leakypipes | fried_rice: k, your patches are now reviewed up to the WIPs. | |
| 21:10:11 | fried_rice | leakypipes Thanks dude. Starting to work through 'em. | |
| 21:10:17 | leakypipes | yup, np | |
| 21:11:52 | openstackgerrit | Matthew Edmonds proposed openstack/nova master: Fix bad autospec usage https://review.openstack.org/535806 | |
| 21:15:21 | edmondsw | claudiub ^ fixed a pep8 error. Interested in how you feel about that patch | |
| 21:16:20 | edmondsw | I'm not against using the fixture you got in oslotests if the performance is better, but this might be quicker to merge and doesn't preclude your changes coming later | |
| 21:16:44 | edmondsw | doesn't have a significant performance impact at this small scale that I've detected | |
| 21:20:26 | edmondsw | cfriesen send_service_user_token is False by default simply because you have to configure credentials for that to work | |
| 21:21:06 | edmondsw | I'd like for send_service_user_token to go away, and just have the code detect whether it has credentials or not and use that as the determining factor, but that's a nit | |
| 21:22:37 | openstackgerrit | Hongbin Lu proposed openstack/nova master: Handle RPC timeout on get_console_output https://review.openstack.org/525335 | |
| 21:28:03 | cburgess | cfriesen Thats very icky, especially considering that its worked this way for a while (the nova side I mean). | |
| 21:28:20 | cburgess | nova merged this stuff shortly after Tokyo | |
| 21:37:56 | cfriesen | cburgess: I think it might be related to the glance v1/v2 stuff | |
| 21:38:06 | cburgess | cfriesen Hmm yeah I think you might be right. | |
| 21:59:39 | openstackgerrit | Jim Rollenhagen proposed openstack/nova master: Ironic: negotiate microversion to allow downgrade to Pike https://review.openstack.org/535786 | |
| 23:00:00 | fried_rice | leakypipes Still around? | |
| 23:04:19 | fried_rice | leakypipes Cancel, I see it now. | |
| 23:28:24 | openstackgerrit | Eric Fried proposed openstack/nova master: Raise on API errors getting aggregates/traits https://review.openstack.org/526540 | |
| 23:28:24 | openstackgerrit | Eric Fried proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756 | |
| 23:28:25 | openstackgerrit | Eric Fried proposed openstack/nova master: Track tree-associated providers in report client https://review.openstack.org/526541 | |
| 23:28:25 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient.set_aggregates_for_provider https://review.openstack.org/533808 | |
| 23:28:26 | openstackgerrit | Eric Fried proposed openstack/nova master: report client: get_provider_tree_and_ensure_root https://review.openstack.org/521098 | |