| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-19 | |||
| 16:09:06 | mdbooth | Hmm, same | |
| 16:09:37 | sean-k-mooney | am why do we do https://github.com/openstack/nova/blob/master/doc/source/conf.py#L201 | |
| 16:10:30 | mdbooth | I don't know, but as that's not eventlet I assume that's ok | |
| 16:10:31 | sean-k-mooney | anyway the only thing we import form nova is nova.version | |
| 16:10:38 | mdbooth | sean-k-mooney: Where do you see that? | |
| 16:10:41 | sean-k-mooney | so before that would not have monkeypatch anything | |
| 16:10:59 | sean-k-mooney | where do i see what? | |
| 16:11:12 | mdbooth | <sean-k-mooney> anyway the only thing we import form nova is nova.version | |
| 16:11:22 | mdbooth | What's your basis for that? | |
| 16:11:55 | sean-k-mooney | oh well i just looked at the conf.py file that sphinx uses i think that is the only pyton file it uses to build the docs | |
| 16:12:03 | sean-k-mooney | im going to do a code search in a second | |
| 16:15:24 | sean-k-mooney | mdbooth: so assuming my github foo hasnt failed me this is the only hit https://github.com/openstack/nova/search?l=Python&q=%22from+nova%22+path%3A%2Fdoc%2Fsource | |
| 16:15:55 | mdbooth | sean-k-mooney: that link doesn't work. | |
| 16:16:00 | mdbooth | What are you looking for? | |
| 16:16:26 | sean-k-mooney | this was the search query in the nova github '"from nova" path:/doc/source' | |
| 16:16:49 | stephenfin | sean-k-mooney: Huh? Context? | |
| 16:16:54 | mdbooth | Ok, sphinx does load_extension() | |
| 16:17:01 | lyarwood | http://codesearch.openstack.org/ - might be easier to search with | |
| 16:17:10 | mdbooth | mod = __import__(extname, None, None, ['setup']) | |
| 16:17:26 | sean-k-mooney | stephenfin: we are trying to figur out what monkeypatch in the toplevel nova module would break the docs job | |
| 16:17:28 | mdbooth | That ends up loading nova/tests/__init__.py, which in my patch does monkey patching | |
| 16:17:43 | mdbooth | Whereas previously it was lower down separately in both unit and functional | |
| 16:17:52 | sean-k-mooney | lyarwood: tried that first but its harder to make look a a single directory | |
| 16:18:08 | mdbooth | Although, why the hell would it load tests and not, for eg nova.cmd or nova.api.openstack? | |
| 16:18:18 | stephenfin | Not an answer but various Sphinx extensions do load the nova source | |
| 16:18:29 | lyarwood | sean-k-mooney: kk | |
| 16:18:39 | stephenfin | sphinx.autodoc and oslo_config.sphinxext jump to mind | |
| 16:18:41 | mdbooth | stephenfin: I'd expect them to load the *whole* nova source though, right? | |
| 16:19:09 | stephenfin | mdbooth: What are they loading? | |
| 16:19:27 | stephenfin | eventlet monkeypatching? | |
| 16:19:32 | stephenfin | or monkeypatching of other things | |
| 16:19:43 | sean-k-mooney | mdbooth: we dont generate docs for the code directly so ingeneral i would have assuemd it should not load the nova code. | |
| 16:19:48 | mdbooth | stephenfin: eventlet. It's done just by loading certain modules. | |
| 16:20:04 | mdbooth | stephenfin: If we import everything, we do monkey patching eventually. | |
| 16:20:20 | stephenfin | We can monkeypatch the monkeypatch though... | |
| 16:20:32 | stephenfin | if that was actually what was happening | |
| 16:20:44 | stephenfin | stub out eventlet, so to speak | |
| 16:20:45 | sean-k-mooney | stephenfin: ew | |
| 16:21:09 | stephenfin | sean-k-mooney: That comment also applies to eventlet itself ;) | |
| 16:21:12 | mdbooth | stephenfin: If it was acceptable we could define an environment variable which, if set, would disable monkey patching | |
| 16:21:19 | mdbooth | However, I'd assumed that was not acceptable | |
| 16:21:28 | jangutter | it would be really funny if rendering the docs accidentally caused an entire Nova deployment to spring up. | |
| 16:21:28 | stephenfin | you'd have to load the os module to do that | |
| 16:21:44 | stephenfin | which I assume eventlet needs to monkeypatch | |
| 16:21:45 | sean-k-mooney | we should not be executing any of the code so we should not need to monkeypatch as monkeypatch should never change any of the api signiture and therefor should nto change the docs | |
| 16:21:51 | sean-k-mooney | in the eventlet case | |
| 16:22:05 | mdbooth | sean-k-mooney: coulda shoulda isn't relevant, though, as it's done at import time | |
| 16:22:09 | sean-k-mooney | mdbooth: we do that for debug right | |
| 16:22:27 | stephenfin | sean-k-mooney: Any code outside a function and not within a 'if __name__ == "__main__"' conditional is going to be executed tho | |
| 16:22:37 | mdbooth | sean-k-mooney: that's done with config | |
| 16:22:56 | mdbooth | sean-k-mooney: Actually by inspecting command line arguments | |
| 16:23:01 | sean-k-mooney | yes but you can now set config values via the environment | |
| 16:23:15 | sean-k-mooney | its new in a recent oslo config | |
| 16:23:34 | mdbooth | def enabled(): | |
| 16:23:34 | mdbooth | return ('--remote_debug-host' in sys.argv and | |
| 16:23:35 | mdbooth | '--remote_debug-port' in sys.argv) | |
| 16:23:42 | mdbooth | ^^^ is the guard | |
| 16:23:50 | sean-k-mooney | yes for debug | |
| 16:24:34 | sean-k-mooney | anyway stephenfin do you know what we use from nova beyond the version and config options for our docs. is that it or do we use other parts of the tree? | |
| 16:24:55 | stephenfin | I'm trying to figure out if we still generate API docs. I don't think we do | |
| 16:25:32 | mdbooth | It's safe to say that this interface is hairy AF, though | |
| 16:25:59 | mdbooth | That is, even if we weren't doing it, somebody only had to sneeze and we were. | |
| 16:27:11 | sean-k-mooney | ya that said i know the docs team wanted to get away from installing proejct deps and loading project in the doc env a while ago. i dont know if that effort did or not but its part of the reason why we created a seperate doc/requirement.txt | |
| 16:27:25 | mdbooth | No, we weren't doing it before | |
| 16:27:40 | sean-k-mooney | monkeypatching? | |
| 16:28:07 | mdbooth | Previously we monkey patched in nova/test.py and nova/tests/functional/__init__.py and sphinx didn't hit that | |
| 16:28:16 | sean-k-mooney | right | |
| 16:28:24 | mdbooth | I moved the test monkey patching to nova/tests/__init__.py, and now it does hit that | |
| 16:28:35 | mdbooth | Go figure | |
| 16:28:53 | mdbooth | If anybody can explain that interface to me I'd be interested | |
| 16:28:56 | sean-k-mooney | does that fix it our recreate teh breakage? | |
| 16:29:11 | mdbooth | I can't reproduce the failure locally, even with monkey patching | |
| 16:29:34 | mdbooth | Ok, I'm just going to move the monkey patching points and add comments not to move them again | |
| 16:30:02 | mdbooth | I have no idea how you're supposed to understand what sphinx imports | |
| 16:30:31 | stephenfin | mdbooth: The whole thing is basically a hack, but there's no easy way to do it | |
| 16:33:40 | sean-k-mooney | stephenfin: the sane thing to do would be not import the nova source when building docs but that is an issue for generationg config docs if we want to do that with sphinx i guess | |
| 16:34:28 | stephenfin | sean-k-mooney: We either do it dynamically, like we do, or we have a tool to export config to a machine readable format (YAML) and parse that | |
| 16:34:50 | stephenfin | and if we go with the latter, updating those definitions now becomes a manual step meaning we need to add tests to ensure it doesn't get out of date | |
| 16:35:16 | mdbooth | stephenfin: But even then presumably the tool which generated the yaml would import the source with a python interpreter | |
| 16:35:21 | jangutter | sean-k-mooney: http://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html "Warning: autodoc imports the modules to be documented. If any modules have side effects on import, these will be executed by autodoc when sphinx-build is run." | |
| 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 | sean-k-mooney | jangutter: i think we are possibly monkeypatching twice | |
| 16:38:37 | mdbooth | sean-k-mooney: I couldn't reproduce, so I have no idea. | |
| 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. | |