| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-21 | |||
| 17:14:54 | sean-k-mooney | dansmith: ah | |
| 17:14:55 | artom | This bare Python file is definitely teh uglies | |
| 17:15:06 | sean-k-mooney | i tought you were going to sugges adding it to the test blacklist | |
| 17:15:13 | sean-k-mooney | but having the test auto skip i think is fine | |
| 17:15:31 | sean-k-mooney | that way we dont need to list a load of special tests if we add more like htis | |
| 17:15:40 | dansmith | right | |
| 17:15:41 | artom | Auto-skip is dangerous, no? Means we'd be silently missing potential errors | |
| 17:15:52 | sean-k-mooney | no in the ci | |
| 17:15:55 | sean-k-mooney | locallly sure | |
| 17:16:03 | dansmith | artom: well, you can also do what the profile test does and set an envar to enable :) | |
| 17:16:05 | sean-k-mooney | but if its run by tox | |
| 17:16:11 | sean-k-mooney | it should be fine | |
| 17:16:39 | artom | dansmith, ah, so folks can disable it if needed | |
| 17:16:50 | artom | Whodda thunk precedent was so useful ;) | |
| 17:16:51 | sean-k-mooney | artom: proably enable if needed | |
| 17:16:52 | dansmith | artom: no, | |
| 17:16:59 | sean-k-mooney | given its racy by defualt | |
| 17:17:12 | dansmith | artom: it's disabled by default, enabled explicitly in that second stestr run command in tox | |
| 17:17:14 | sean-k-mooney | you would set the env var wehn you run it explcitly | |
| 17:17:25 | sean-k-mooney | yep what dansmith said | |
| 17:17:28 | artom | dansmith, sorry, right | |
| 17:17:49 | artom | I meant in the sense of... tox will run with it enabled | |
| 17:18:03 | artom | Everything else won't | |
| 17:18:06 | sean-k-mooney | tox -e py<whatever> sure | |
| 17:18:19 | dansmith | seriously, there are a bunch of other things in that file with comments like "this is fragile if order isn't respected" comments... it'd be nice to get tests for those things too since we're clearly just flying by the seat of our pants on these things | |
| 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 | openstack | Launchpad bug 1773102 in OpenStack Compute (nova) queens "Abnormal request id in logs" [Medium,Fix committed] - Assigned to Radoslav Gerganov (rgerganov) | |
| 17:22:41 | sean-k-mooney | https://bugs.launchpad.net/nova/+bug/1773102 | |
| 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 | openstack | Launchpad bug 1773102 in OpenStack Compute (nova) queens "Abnormal request id in logs" [Medium,Fix committed] - Assigned to Radoslav Gerganov (rgerganov) | |
| 17:24:53 | dansmith | https://bugs.launchpad.net/nova/+bug/1773102/comments/18 | |
| 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 | dansmith | ack | |
| 17:45:44 | sean-k-mooney | so i think its right | |
| 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 | |