| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-02-17 | |||
| 21:15:38 | johnsom | backport candidate is a tool the Octavia cores are using to track patches for backport to stable branches. We added that for our repos. | |
| 21:16:02 | johnsom | Since.... Storyboard...... Enough said | |
| 21:16:29 | efried | johnsom: I was poking you about whether there's a known bug opened against mock for this | |
| 21:16:33 | sean-k-mooney | efried: what we could do is update the requriements file only install mock on python2 | |
| 21:17:17 | johnsom | efried I burned my morning trying to fix a cause and or fix for this. No luck. I'm also not sure this is the same issue your project has with the UC changes. | |
| 21:17:21 | sean-k-mooney | we would then need to update all the projects. | |
| 21:17:25 | efried | sean-k-mooney: not sure that helps, | |
| 21:17:25 | efried | But taking that idea... | |
| 21:17:25 | efried | It might be possible to say | |
| 21:17:25 | efried | mock;python!=3.xx | |
| 21:17:46 | efried | johnsom: yeah, valid point, I didn't actually look. | |
| 21:17:53 | johnsom | We are having the same discussion, whether we should be using this package or reverting to the built-in now. | |
| 21:17:54 | sean-k-mooney | ya so we have a way to match based on python version in the requeriments file already | |
| 21:18:35 | efried | well, fwiw, simply removing mock from test-requirements didn't stop it from installing in the venv. Imagine we would have to yank it from oslotest and oslo.vmware at least. | |
| 21:18:37 | sean-k-mooney | efried: i can ask the requirements folk about it tomorrow seperate form the current disucssion | |
| 21:18:52 | sean-k-mooney | efried: yes we would | |
| 21:19:08 | sean-k-mooney | efried: if you mannually remove it form the enve does it resovel the issue | |
| 21:19:18 | efried | right -- prometheanfire fyi ^ unless you know a way we can explicitly prevent mock from installing, we'll still get it from transitive if we just remove it from our own test-requirements. | |
| 21:19:41 | johnsom | Agreed, it is a total hack, but it was only in "legacy" code for us, so we were like.... good enough, it's going to die anyway. | |
| 21:20:06 | prometheanfire | heh, that's a good point | |
| 21:20:35 | prometheanfire | efried: I don't know of one yet | |
| 21:21:25 | efried | johnsom: I totally wasn't judging you btw :P | |
| 21:21:33 | sean-k-mooney | prometheanfire: have you confirmed if removing the lib version resolved the issue | |
| 21:21:40 | sean-k-mooney | also what is the issue? | |
| 21:22:14 | efried | https://ad2781ccb869c0ef3e99-0fce708e89812c28d28e2be1213f1c9b.ssl.cf5.rackcdn.com/707064/10/check/cross-nova-py36/d1c7fa6/testr_results.html | |
| 21:22:21 | prometheanfire | I don't have a link to an issue yet, this is bigger than I first though that it was | |
| 21:22:52 | johnsom | Yeah, your logs don't look like the same issue we had | |
| 21:23:35 | sean-k-mooney | "ValueError: Sentinels must not start with _" ? | |
| 21:23:40 | johnsom | Ah, ok, some are kind of similar | |
| 21:23:50 | johnsom | Yeah, that appears to be on a "spec" mock | |
| 21:23:52 | prometheanfire | heh https://github.com/testing-cabal/mock/pull/482 | |
| 21:25:29 | sean-k-mooney | right so we could just replace all uses of import mock with import unittest.mock? | |
| 21:25:45 | sean-k-mooney | to get rid of the dep on mock the lib | |
| 21:25:53 | efried | "just" | |
| 21:25:56 | sean-k-mooney | if we wanted too in the futrue and add a hacking check | |
| 21:26:24 | sean-k-mooney | efried: its hopefully 1 sed command but yes | |
| 21:26:29 | sean-k-mooney | it could explode | |
| 21:26:43 | efried | 384 occurrences. | |
| 21:27:38 | prometheanfire | well, there's a way to find out I guess | |
| 21:28:06 | efried | yeah, working on it. | |
| 21:28:57 | sean-k-mooney | there has been a long standing issue in nova where we cant use self.assertRaises as a context manager in a with statement which im pretty sure is caused by mock the lib by the way whic this might also fix | |
| 21:29:07 | efried | prometheanfire, sean-k-mooney, johnsom: so yeah, that fixes it, at least for the one instance I tried. | |
| 21:29:21 | efried | s/^import mock$/from unittest import mock/ | |
| 21:29:25 | prometheanfire | heh | |
| 21:29:36 | johnsom | Hmmm, so tempting..... grin | |
| 21:29:55 | sean-k-mooney | can you do that for all nova and see if the tests still run | |
| 21:29:59 | johnsom | Yeah, I think I will try that too | |
| 21:30:04 | efried | sean-k-mooney: yeah but I'm not gonna run locally. | |
| 21:30:11 | efried | My laptop will burn a hole to the center of the earth. | |
| 21:30:14 | efried | I'll let zuul do it. | |
| 21:30:19 | sean-k-mooney | ok | |
| 21:30:22 | johnsom | It's a mock revolt. lol | |
| 21:31:01 | sean-k-mooney | if it does have issue i guess we could just do it selectivly to solve the issues we currently have until either mock is fixt or we fix the issues we find | |
| 21:32:01 | efried | sean-k-mooney: the sed command works fine... except for the pep8 requirement to sort imports :P | |
| 21:32:28 | sean-k-mooney | autopep8 might fix that for you | |
| 21:32:50 | sean-k-mooney | unlike black it only fixes the minium amount of stuff to pass pep8 | |
| 21:33:00 | sean-k-mooney | so it does not break the worlds and passes our gate | |
| 21:33:47 | sean-k-mooney | anyway we coudl cross that bridge when we see if it even passes the zuul run | |
| 21:37:22 | efried | prometheanfire: having proven this is a bug in the mock library, are you more convinced that we should blacklist the mock versions? | |
| 21:38:13 | sean-k-mooney | got to love that commit message https://github.com/testing-cabal/mock/commit/9e5e0380626fd3c540aa4799df0e794cf24d16aa | |
| 21:38:34 | prometheanfire | efried: ya, kinda sucks, but ok | |
| 21:38:39 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Use stock mock https://review.opendev.org/708262 | |
| 21:38:56 | efried | prometheanfire: if you want to try making your patch dep on ^ ... | |
| 21:39:13 | efried | sean-k-mooney: --^ | |
| 21:39:14 | johnsom | Yeah, octavia seems to pass fine with the native mock | |
| 21:39:39 | efried | o queso we need a bug against mock | |
| 21:40:23 | efried | am I reading this right that there are zero open issues? | |
| 21:40:28 | efried | am I looking at the right repo? | |
| 21:40:44 | sean-k-mooney | and 404 closed | |
| 21:40:58 | johnsom | Zero for that repo | |
| 21:41:13 | johnsom | Though they pull in from the native bugs too | |
| 21:41:40 | sean-k-mooney | i guess they just fix them quickly or close them as wont fix | |
| 21:41:42 | prometheanfire | efried: well, for me I'm blacklisting on the reqs side, not nova side | |
| 21:41:51 | sean-k-mooney | e.g. they dont leave them open for log | |
| 21:41:54 | sean-k-mooney | *long | |
| 21:43:42 | efried | prometheanfire: I assume we would like to be able to continue following upgrades here, given that trying to fix the transitive deps through all of openstack would be hard and take long. So we should still open an issue for this. Agree? | |
| 21:44:15 | sean-k-mooney | efried: if it broke us it will proably break others | |
| 21:44:21 | sean-k-mooney | so its worth reporting in any case | |
| 21:44:23 | prometheanfire | efried: ya, definitely | |
| 21:46:46 | efried | prometheanfire, sean-k-mooney, johnsom: https://github.com/testing-cabal/mock/issues/487 | |
| 21:47:02 | prometheanfire | watching, thanks | |
| 21:47:14 | prometheanfire | I'll submit a blacklist review to the reqs repo in a few min | |
| 21:54:15 | prometheanfire | submitted and updated the gr update review | |
| 21:54:26 | sean-k-mooney | oh looking at that traceback this looks like our error | |
| 21:55:15 | sean-k-mooney | the reason its passing on python 3.7 is _is_coroutine was likely added by python 3.8 | |
| 21:56:26 | efried | johnsom: want to throw https://zuul.opendev.org/t/openstack/build/c5fd6cadc9bf4b3b9ea81372ef047246/log/job-output.txt#4605 in there? | |
| 21:56:51 | efried | sean-k-mooney: what do you mean passing on py37? It failed locally for me on py37 | |
| 21:57:06 | sean-k-mooney | with unittest.mock | |
| 21:57:08 | johnsom | Yeah, I can | |
| 22:04:18 | sean-k-mooney | efried: basiacly i think we are asserting that that the attibute does not start with _ incorrectly here https://github.com/openstack/oslo.utils/blob/master/oslo_utils/fixture.py#L81-L82 | |
| 22:04:51 | sean-k-mooney | or actully no maybe that is not the issue | |
| 22:05:20 | sean-k-mooney | that is where the exctip is being raised but i have looked at why we are calling that code | |
| 22:05:22 | efried | yeah, the issue is that the wrong `name` is now getting to that point. | |
| 22:05:46 | efried | probably '_AutoSpecMagicMock' as opposed to the name of the actual sentinel. | |
| 22:06:35 | sean-k-mooney | ya | |
| 22:06:47 | sean-k-mooney | so i was wondering is this a change in python 3.8 | |
| 22:06:55 | sean-k-mooney | that they have now backported to mock the lib | |
| 22:07:02 | sean-k-mooney | or is it a mock the lib issue | |
| 22:07:42 | efried | sean-k-mooney: do you have 3.8 installed somewhere? Easy enough to try it out. | |
| 22:08:29 | efried | Just | |