| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-09-21 | |||
| 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 | |
| 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 | sean-k-mooney | fucntion defietly wont | |
| 17:49:49 | dansmith | functional moneky patches in __init__ ;P | |
| 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 | artom | So it's fine if it gets put in nova/tests | |
| 17:51:46 | dansmith | and then it will be excluded from the regular stest execution anyway, | |
| 17:51:50 | dansmith | which is what we wanted right? | |
| 17:51:52 | sean-k-mooney | drop it at the top level and run it explcitly with tox | |
| 17:51:55 | dansmith | yah | |
| 17:52:29 | sean-k-mooney | ya it is | |
| 17:53:43 | artom | Don't need the env var skip mechanism then, either | |
| 17:53:50 | dansmith | right | |
| 17:55:26 | sean-k-mooney | so this http://paste.openstack.org/show/798162/ | |
| 17:55:57 | sean-k-mooney | actully setp is not needed | |
| 17:56:07 | dansmith | yup | |
| 17:56:34 | dansmith | but it'd be nice to get some other validations in there for the other things if we can | |
| 17:56:54 | sean-k-mooney | ya im not super happy with the self.assertNotIn('.greendns', tb) | |
| 17:56:56 | dansmith | artom: also, my -1 is going to be "change the title of the patch to what it does and not an obscure 'tests for $hash' message" | |
| 17:57:20 | artom | dansmith, fair | |
| 17:57:27 | artom | sean-k-mooney, I'll add comments? | |
| 17:57:29 | dansmith | lots of people do that, but I hate it | |
| 17:57:59 | artom | Tests for hash is what I do on the weekends :D | |
| 17:58:07 | sean-k-mooney | actully it should be greendns.py | |
| 17:58:18 | sean-k-mooney | its not the module path in the trace its the file path | |
| 17:58:19 | dansmith | artom: it's legal here now, so doesn't make you seem cool and edgy like it used to | |
| 17:59:19 | artom | dansmith, but what will I do for personality now? | |
| 18:01:42 | dansmith | dirty jokes? | |
| 18:01:55 | sean-k-mooney | so this works for me now that i moved it http://paste.openstack.org/show/798163/ | |
| 18:03:22 | sean-k-mooney | oh this is what i ran http://paste.openstack.org/show/798164/ | |
| 18:21:26 | openstackgerrit | Merged openstack/nova stable/rocky: libvirt: Handle VIR_ERR_DEVICE_MISSING when detaching devices https://review.opendev.org/742417 | |
| 18:26:23 | CeeMac | Evening all | |