| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-19 | |||
| 16:35:25 | sean-k-mooney | there is prably an option 3 | |
| 16:35:28 | stephenfin | autodocing APIs is even more complicated since you do need a full environment to be able to do introspection | |
| 16:35:33 | mdbooth | stephenfin: So we'd just be moving the problem elsewhere | |
| 16:36:00 | stephenfin | mdbooth: Correct, but that's sometimes a reasonable thing to do | |
| 16:36:19 | sean-k-mooney | jangutter: ya im aware of how autodocs works. | |
| 16:36:44 | sean-k-mooney | jangutter: its both good and bad | |
| 16:37:07 | sean-k-mooney | jangutter: we have some module level stuff we caluate on import that is slow | |
| 16:37:19 | stephenfin | mdbooth: We can either store this stuff as code and deal with these dependency issues or we store it separately and then deal with keeping it up-to-date. Choose one | |
| 16:38:08 | stephenfin | (I'm very much in favour of the former, as noted by my work on (cliff|oslo_config|oslo_policy).sphinxext | |
| 16:38:11 | stephenfin | *) | |
| 16:38:12 | sean-k-mooney | mdbooth: did you determin if the issue is with monkey patching twice or not monkey patch at all or didnt find the root cause | |
| 16:38:14 | jangutter | sean-k-mooney: and the current theory is that autodoc and monkeypatching are fighting? | |
| 16:38:37 | mdbooth | sean-k-mooney: I couldn't reproduce, so I have no idea. | |
| 16:38:37 | sean-k-mooney | jangutter: i think we are possibly monkeypatching twice | |
| 16:38:47 | sean-k-mooney | mdbooth: i can repoduce locally | |
| 16:38:55 | mdbooth | sean-k-mooney: On Ubuntu? | |
| 16:38:59 | stephenfin | jangutter: Not necessarily autodoc. Probably oslo_config.sphinxext | |
| 16:39:04 | sean-k-mooney | yes.. | |
| 16:39:06 | stephenfin | Don't think we do autodoc | |
| 16:39:07 | jangutter | mdbooth: I reproduced on a CentOS 7 container. | |
| 16:39:28 | sean-k-mooney | ok so it cross distro | |
| 16:39:30 | jangutter | stephenfin: ah! | |
| 16:39:47 | stephenfin | mdbooth: Good reading, fwiw http://python-notes.curiousefficiency.org/en/latest/python_concepts/import_traps.html | |
| 16:39:49 | sean-k-mooney | mdbooth: we should proably open a bug and fix it later | |
| 16:39:56 | mdbooth | That seems to avoid whatever sphinx is importing | |
| 16:40:58 | mdbooth | sean-k-mooney: For what? | |
| 16:41:49 | stephenfin | mdbooth: It's loading the module. I suspect you'd get the same issue if you left it in 'nova/test.py' and added 'import .test' to 'nova/tests/__init__.py' | |
| 16:41:57 | stephenfin | *'import nova.test' | |
| 16:42:45 | mdbooth | stephenfin: Which module is it loading? | |
| 16:43:16 | stephenfin | It's loading 'nova', which is causing it to load 'nova.tests', I would imagine | |
| 16:43:26 | sean-k-mooney | mdbooth: https://review.openstack.org/#/c/626952/8/nova/monkey_patch.py@55 | |
| 16:43:38 | sean-k-mooney | mdbooth: does that comment make sense. | |
| 16:43:59 | sean-k-mooney | actull i test it locally | |
| 16:45:29 | stephenfin | and that's happening because of https://github.com/openstack/nova/blob/master/doc/source/conf.py#L27 | |
| 16:45:33 | mdbooth | sean-k-mooney: Right, if the problem is importing twice that would work round it. However, if we previously weren't monkey patching at all by happy coincidence, I'm not sure if that's not just a dirtier hack. | |
| 16:45:49 | stephenfin | though that's clearly not necessary https://github.com/openstack/nova/blob/master/doc/source/conf.py#L22 | |
| 16:46:55 | mdbooth | stephenfin: “Never add a package directory, or any directory inside a package, directly to the Python path”. | |
| 16:47:17 | mdbooth | From that traps doc you sent earlier | |
| 16:48:08 | stephenfin | yuuuup | |
| 16:48:23 | stephenfin | In general, messing with the Python path is a bad idea | |
| 16:48:39 | stephenfin | but the alternative is making all these things installable | |
| 16:48:44 | mdbooth | s/(the|path) // | |
| 16:49:01 | mdbooth | g | |
| 16:49:17 | stephenfin | :) | |
| 16:49:24 | stephenfin | I like to live on the edge, personally | |
| 16:50:46 | mdbooth | Ok, I'm going to do both | |
| 16:51:12 | mdbooth | I'm going to move the monkey patching points back to where they were, *and* add a guard to explicitly avoid double monkey patching | |
| 16:52:29 | stephenfin | mdbooth: I think you might also be interested in some of Armin Ronacher's most recent presentations on all things Python http://lucumr.pocoo.org/talks/ | |
| 16:52:39 | sean-k-mooney | mdbooth: i tryed the guard locally and it did not seam to help | |
| 16:52:40 | stephenfin | he of Flask fame | |
| 16:54:09 | stephenfin | ...openstack/nova/doc/source/reference/notifications.rst:142:Unknown directive type "automodule". | |
| 16:54:14 | stephenfin | So we are using autodoc | |
| 16:58:00 | stephenfin | mdbooth: Try adding this to doc/source/conf.py | |
| 16:58:07 | stephenfin | autodoc_mock_imports = ['nova.tests'] | |
| 16:59:21 | mdbooth | stephenfin: That's still really flaky, though, because it doesn't explain why we don't import the nova.cmd or nova.api.openstack.wsgi_app entry points | |
| 16:59:39 | mdbooth | stephenfin: I'd prefer to either go from flaky to non-flaky, or just leave well alone | |
| 17:00:09 | mdbooth | flaky->different flaky isn't worth the churn, I think | |
| 17:00:45 | sean-k-mooney | mdbooth: im flipped the condition on my local gard to never monkey patch and that fixes it | |
| 17:00:59 | sean-k-mooney | so the issue is the docs only build if we never monkeypatch | |
| 17:01:05 | stephenfin | Oh, I didn't realize we did that there | |
| 17:01:34 | sean-k-mooney | stephenfin: we monkey patch on the __init__.py in nova/cmd | |
| 17:01:40 | stephenfin | Yup, see that now | |
| 17:01:45 | stephenfin | weirdddd | |
| 17:02:03 | stephenfin | mdbooth: Can you share the debug logs from Sphinx? | |
| 17:02:19 | sean-k-mooney | ya anywya as i said above the issue is the sphinx only works if we do not do eventlet.monkeypatch | |
| 17:02:24 | mdbooth | stephenfin: In what configuration? | |
| 17:02:39 | mdbooth | stephenfin: FWIW, this doesn't fail for me on F29, even when monkey patched | |
| 17:03:02 | mdbooth | Although apparently it does fail on Centos7 and Ubuntu, so I'm guessing it's some library version | |
| 17:03:02 | stephenfin | Oh, wherever it's failing | |
| 17:03:11 | stephenfin | I assume that's only on Ubuntu so? | |
| 17:03:12 | stephenfin | Ack | |
| 17:03:36 | sean-k-mooney | stephenfin: jangutter mentioned its broken on centos 7 too | |
| 17:03:56 | sean-k-mooney | mdbooth: are you running on py36 on fedora 29 or py37 | |
| 17:04:07 | sean-k-mooney | ubuntu and centos 7 use py36 | |
| 17:04:16 | stephenfin | Yeah, we need debug logs for this stuff | |
| 17:04:19 | mdbooth | 37 | |
| 17:04:39 | sean-k-mooney | ok so it could be a 36 vs 37 thing | |
| 17:05:08 | openstackgerrit | Lance Bragstad proposed openstack/nova master: Give the policy vision document a facelift https://review.openstack.org/644615 | |
| 17:05:36 | sean-k-mooney | ill install 3.7 and check | |
| 17:05:54 | lbragstad | mriedem lumped it all in there ^ lemme know if you want that broken up | |
| 17:06:00 | lbragstad | cc efried ^ | |
| 17:07:19 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Misc cleanups https://review.openstack.org/644616 | |
| 17:10:27 | sean-k-mooney | so looks like ti works fine on python 3.7 with the gauard | |
| 17:10:45 | sean-k-mooney | ill revert to a clean copy of the patch and confrim its fine without the guard too | |
| 17:13:19 | mdbooth | Hehe, with vim's standard highlighting in nova.monkey_patch, I see: NOTE, NOTE, TODO, NOTE, NOTE, NOTE, NOTE | |
| 17:13:27 | aspiers | sean-k-mooney: we got Brijesh from AMD to respond to your hugepages suggestion | |
| 17:13:37 | sean-k-mooney | mdbooth: stephenfin so yes this only happens on python3.6 which is why it passes on fedora and not on ubuntu or centos which default to python 3.6 | |
| 17:13:50 | mdbooth | Well, at least they're all in the same place, now | |
| 17:13:53 | sean-k-mooney | aspiers: oh ok i havent read it yet | |
| 17:14:06 | sean-k-mooney | aspiers: was it favorable or not? | |
| 17:14:20 | aspiers | sean-k-mooney: I don't think so. Bottom of https://review.openstack.org/#/c/641994/2 | |
| 17:15:02 | aspiers | SEV by design prevents the kernel copying encrypted pages around | |
| 17:15:34 | aspiers | he says using hugepages would pin it within the hugepages pool, but not pin each page to a single address | |
| 17:15:44 | aspiers | I'm on a call with AMD now | |
| 17:16:48 | sean-k-mooney | aspiers: ya dan berrange replied and said hugepages may not be sufficent too as the roms and uefi pflash would not be pinned | |
| 17:16:55 | aspiers | right | |
| 17:18:04 | aspiers | sean-k-mooney: how does it work for device passthrough? | |
| 17:18:47 | aspiers | apparently that's a similar scenario in terms of pinning | |
| 17:20:34 | sean-k-mooney | the iommu maps the device mmio region to a fixed set of addresses that are maped to the memory regions listed in the pci VF config space i think. but honestly i tought that a hugepage was similarly fixed and could not migrate when we use explcit not transparent hugepages | |
| 17:21:30 | mdbooth | It seems there's no need for a double-import guard unless you borked the python path, because python doesn't appear to import a second time anyway | |