| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-21 | |||
| 17:18:26 | sean-k-mooney | but in two calls to stester | |
| 17:18:47 | artom | Wait, no, that second tox command that I'll add will enable and run it | |
| 17:18:53 | artom | Nothing else will because it's racy | |
| 17:19:01 | dansmith | urllib3, oslo_context, threading things, | |
| 17:19:10 | dansmith | artom: right, exactly.. you know, like the profiling test :) | |
| 17:19:21 | sean-k-mooney | dansmith: ya we have a cople of example of this | |
| 17:19:39 | artom | dansmith, you keep saying that like I'm some sort of genius that understands it the first time around | |
| 17:19:41 | artom | ;) | |
| 17:19:53 | sean-k-mooney | not sure why oslo_context would be on that list but why not | |
| 17:19:57 | dansmith | artom: it just makes me feel better to highlight the first-mention-to-grok time gap :) | |
| 17:20:09 | dansmith | sean-k-mooney: maybe if we had a test with comments in it, we'd know :/ | |
| 17:20:33 | artom | dansmith, I'm pretty sure it's exponential with age (and inverse with sleep ) | |
| 17:20:40 | sean-k-mooney | :) well it was more oslo is an openstack thing so hopefully we could fix the oslo issue there instead of nova | |
| 17:20:47 | dansmith | artom: we should plot it on a graph | |
| 17:21:14 | dansmith | sean-k-mooney: not likely if it's something oslo can't do if imported post-monkey-patch | |
| 17:21:16 | artom | ∞ there's your graph | |
| 17:21:19 | dansmith | heh | |
| 17:22:41 | sean-k-mooney | https://bugs.launchpad.net/nova/+bug/1773102 | |
| 17:22:41 | openstack | Launchpad bug 1773102 in OpenStack Compute (nova) queens "Abnormal request id in logs" [Medium,Fix committed] - Assigned to Radoslav Gerganov (rgerganov) | |
| 17:22:51 | sean-k-mooney | thats the oslo_context bug | |
| 17:23:20 | dansmith | ah yeah, and surely that's because it needs to grab pointers to the threading primitives for TLS before they get munched right? | |
| 17:24:06 | sean-k-mooney | maybe although there might be a way to resolve that in a differnt way. anyway not important right now | |
| 17:24:51 | sean-k-mooney | artom: so you going to add a second run combining the results with teh first and explictly enable that test in the second run ya | |
| 17:24:53 | dansmith | https://bugs.launchpad.net/nova/+bug/1773102/comments/18 | |
| 17:24:53 | openstack | Launchpad bug 1773102 in OpenStack Compute (nova) queens "Abnormal request id in logs" [Medium,Fix committed] - Assigned to Radoslav Gerganov (rgerganov) | |
| 17:25:00 | dansmith | it'd be hard to solve it any other way, but.. yah | |
| 17:25:35 | artom | dansmith, for all this talk of "do it like the profiler", I've tried it, and looks like it's still importing eventlet elsewhere | |
| 17:26:20 | sean-k-mooney | artom: can you push it so we can try it | |
| 17:29:04 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Unit test for 7c1d964faa https://review.opendev.org/753072 | |
| 17:29:08 | artom | sean-k-mooney, ^^ | |
| 17:31:38 | dansmith | artom: can you try it with python3 -munittest path.to.test ? | |
| 17:32:35 | dansmith | artom: wait, you're still importing nova.test in your module | |
| 17:32:36 | dansmith | that means you do it before setUp() runs :) | |
| 17:33:00 | sean-k-mooney | yep that would do it | |
| 17:33:04 | dansmith | so that "importing eventlet elsewhere" is ... in your test :) | |
| 17:33:52 | artom | Right, but nova.test imports eventlet *after setting the greendns env var* | |
| 17:34:01 | artom | So that should be fine | |
| 17:34:21 | artom | The problem is if some other unit tests straight up `import eventlet` without setting EVENTLET_NO_GREENDNS | |
| 17:34:30 | dansmith | I thought something else does that is downstream of that, and that was the whole point? | |
| 17:34:46 | artom | Something else does what? | |
| 17:34:58 | dansmith | import eventlet | |
| 17:35:24 | sean-k-mooney | that what we get when we run it directly http://paste.openstack.org/show/798154/ | |
| 17:36:23 | artom | Weird | |
| 17:38:11 | sean-k-mooney | basically we need to run this test using unitt test and not the base testcase and only import monkey patch inside teh test funtion i think | |
| 17:38:19 | sean-k-mooney | im going to try that locally | |
| 17:38:41 | dansmith | right | |
| 17:38:58 | dansmith | although I moved the import into the test function and I still fail the test | |
| 17:39:27 | dansmith | so, here's the other thing | |
| 17:39:41 | dansmith | stestr forks and runs the worker children outside the main process | |
| 17:39:57 | dansmith | which may not share the environment where the flag is being set | |
| 17:40:09 | artom | Sounds "right" - in the sense that, if we're the *only* ones importing nova.test, and that imports eventlet "properly" (setting EVENTLET_NO_GREENDNS), shouldn't matter when we do the importing | |
| 17:41:04 | dansmith | still happens with bare unittest run | |
| 17:41:27 | sean-k-mooney | http://paste.openstack.org/show/798157/ | |
| 17:42:07 | sean-k-mooney | so it match on the function call | |
| 17:42:24 | artom | *facepalm* | |
| 17:42:49 | sean-k-mooney | test_greendns_is_disabled | |
| 17:42:50 | dansmith | ah hah | |
| 17:43:46 | dansmith | although I'm seeing it for realz: /home/dan/nova/.tox/py37/lib/python3.7/site-packages/eventlet/support/greendns.py | |
| 17:43:47 | artom | Maybe premature though, changing it go 'greendns.py' still fails | |
| 17:44:12 | dansmith | any chance the flag name is wrong, or requires a value different than "yes" ? | |
| 17:44:47 | artom | dansmith, I don't think it's that - remember, the bare python file worked | |
| 17:44:56 | artom | worked == verified the correct behaviour | |
| 17:45:04 | sean-k-mooney | dansmith https://github.com/eventlet/eventlet/blob/af407c77f208ceefe5a35e39aed0cf3fdfc07cb9/eventlet/green/socket.py#L20 | |
| 17:45:11 | sean-k-mooney | if os.environ.get("EVENTLET_NO_GREENDNS", '').lower() != 'yes': | |
| 17:45:44 | sean-k-mooney | so i think its right | |
| 17:45:44 | dansmith | ack | |
| 17:46:32 | sean-k-mooney | so i have a working copy locally | |
| 17:46:40 | sean-k-mooney | just changed it to greendns.py | |
| 17:46:57 | sean-k-mooney | but is that goign to be in the traceback | |
| 17:47:18 | sean-k-mooney | http://paste.openstack.org/show/798159/ | |
| 17:47:29 | dansmith | https://pastebin.com/kY9iCQpu | |
| 17:47:33 | dansmith | this is how I'm getting to eventlet ^ | |
| 17:47:57 | dansmith | unit/__init__.py calls objects.register_all() | |
| 17:47:59 | dansmith | and eventlet is in there | |
| 17:48:10 | sean-k-mooney | ya i was looking at that | |
| 17:48:19 | sean-k-mooney | i didnt see which object it was | |
| 17:48:34 | dansmith | so, like I surmised, something in the test infra is hitting it before we even get to us, but unfortunately it's in our module so it gets imported before we get imported | |
| 17:48:36 | artom | Oh, smart just raising in evenlet itself and examining the trace | |
| 17:48:42 | dansmith | if it wasn't in an __init__ then we'd be safe | |
| 17:48:43 | sean-k-mooney | nova.objects.agent | |
| 17:49:18 | artom | dansmith, so then... if I move it out of tests/unit ? | |
| 17:49:31 | dansmith | artom: yeah I guess.. maybe functional would work by chance? | |
| 17:49:41 | dansmith | lol, | |
| 17:49:44 | sean-k-mooney | no | |
| 17:49:49 | dansmith | functional moneky patches in __init__ ;P | |
| 17:49:49 | sean-k-mooney | fucntion defietly wont | |
| 17:49:54 | sean-k-mooney | yep | |
| 17:49:57 | artom | dansmith, no, that as import nova.monkey_patch # noqa in __init__.py | |
| 17:50:00 | artom | *has | |
| 17:50:13 | sean-k-mooney | also fucntion is still under nova.test | |
| 17:50:17 | sean-k-mooney | which import the objects | |
| 17:50:29 | sean-k-mooney | i would have to be nova.evetlet.tests or soemthign like that | |
| 17:50:57 | dansmith | sean-k-mooney: it's tests/unit/__init not tests/__init | |
| 17:51:08 | sean-k-mooney | oh your right | |
| 17:51:10 | dansmith | but yeah, unless we move that patch out of functional/__init it doesn't matter | |
| 17:51:26 | sean-k-mooney | then ya just tests/ then | |
| 17:51:33 | dansmith | yeah, | |
| 17:51:46 | dansmith | and then it will be excluded from the regular stest execution anyway, | |
| 17:51:46 | artom | So it's fine if it gets put in nova/tests | |
| 17:51:50 | dansmith | which is what we wanted right? | |