| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-12-09 | |||
| 09:55:39 | lyarwood | elod: I *think* that's enough right? | |
| 09:56:52 | elod | thanks! setup.cfg looks ok ( https://github.com/PyCQA/bandit/blob/1.6.3/setup.cfg ) so I think this patch with the cap should be OK | |
| 09:57:16 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: libvirt: Skip encryption metadata lookups if secret already exists on host https://review.opendev.org/c/openstack/nova/+/765771 | |
| 09:57:26 | lyarwood | kk testing above | |
| 09:59:38 | elod | maybe we need to add that to global-requirements.txt | |
| 10:00:26 | lyarwood | hmm it's only a test-requirment in nova | |
| 10:04:53 | elod | lyarwood: it seems the patch won't work: https://opendev.org/openstack/requirements/src/branch/stable/train/blacklist.txt#L5 | |
| 10:05:32 | elod | so if I understand correctly we need to add it to every branch in every test-requirements.txt :/ | |
| 10:05:50 | elod | * the bandit cap | |
| 10:06:24 | lyarwood | elod: in nova right? | |
| 10:06:28 | lyarwood | fun | |
| 10:06:30 | elod | yes | |
| 10:06:33 | lyarwood | okay I'll do that now | |
| 10:07:39 | elod | (and "fortunately" I see lots of bandit failures all along other openstack repos :/ so it will be a nice amount of bandit patch if I'm not mistaken...) | |
| 10:08:26 | stephenfin | lyarwood: elod: We should probably move bandit and other linters out of test-requirements.txt and into tox.ini since they're not subject to upper-constraints | |
| 10:08:56 | stephenfin | https://github.com/openstack/python-openstackclient/blob/master/tox.ini#L31-L34 | |
| 10:09:04 | stephenfin | from https://github.com/openstack/python-openstackclient/commit/20769cd7b27d51da84a324a17922427eba5c6eac | |
| 10:09:34 | lyarwood | stephenfin: we can start doing that on master | |
| 10:09:44 | lyarwood | stephenfin: I'm not sure we want to change that on stable however right? | |
| 10:10:26 | stephenfin | I wouldn't see a reason not to, assuming your issue is derived from the new pip 20.3 resolver, rather than simply uncapped requirements | |
| 10:10:47 | stephenfin | If it's the latter, obviously just cap them and be done with it, sure | |
| 10:11:16 | lyarwood | it's the latter sorry, I assumed it was pip to begin with | |
| 10:11:22 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: Cap bandit at 1.6.2 when using py2 https://review.opendev.org/c/openstack/nova/+/766171 | |
| 10:11:28 | stephenfin | ah, then yes, cap all the way | |
| 10:11:29 | lyarwood | but it just wasn't capped | |
| 10:13:01 | lyarwood | okay lets try this again | |
| 10:13:06 | elod | hmmm, stephenfin I don't see the benefit of movint from test-req to tox.ini. we use capping in test-req, too | |
| 10:13:18 | stephenfin | elod: not for linters, you don't | |
| 10:13:31 | stephenfin | https://github.com/openstack/requirements/blob/master/blacklist.txt | |
| 10:13:38 | lyarwood | as we just found out :) | |
| 10:13:40 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: libvirt: Skip encryption metadata lookups if secret already exists on host https://review.opendev.org/c/openstack/nova/+/765771 | |
| 10:13:59 | elod | but that just mean there shouldn't be a global cap | |
| 10:14:04 | elod | in test-req it's OK | |
| 10:14:12 | elod | at least this is how I understand :) | |
| 10:14:32 | stephenfin | Hmm, I recall seeing something from mordred about this a while ago on openstack-discuss. Wonder if I can find it... | |
| 10:16:28 | lyarwood | actually let me write up a bug for this | |
| 10:16:29 | stephenfin | elod, lyarwood: Okay, this is what I was thinking of http://lists.openstack.org/pipermail/openstack-discuss/2020-March/013642.html | |
| 10:17:05 | elod | just found it, too, but have to re-read :) | |
| 10:17:27 | stephenfin | Not exactly it, but it does describe the issue I was seeing with OSC. Specifically "That means we uninstall and reinstall flake8 at different versions over and over again - and the final state is not one that is completely consistent." | |
| 10:17:39 | stephenfin | pip went nuts when those were included | |
| 10:18:24 | stephenfin | With that said, we have out own hacking plugins and tests for same, which means we do actually depend on those things to run unit tests. I don't know what the answer for that is :-\ | |
| 10:18:41 | stephenfin | Maybe it's just "Not A Problem" (TM)? :) | |
| 10:20:20 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: Cap bandit at 1.6.2 when using py2 https://review.opendev.org/c/openstack/nova/+/766171 | |
| 10:20:21 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: libvirt: Skip encryption metadata lookups if secret already exists on host https://review.opendev.org/c/openstack/nova/+/765771 | |
| 10:20:27 | lyarwood | oh wait, so adding the cap in test-reqs isn't going to work? | |
| 10:20:48 | elod | the uninstall and reinstall is mainly a problem in devstack. but test-req installation is removed in devstack, so that's not a problem anymore. | |
| 10:21:23 | elod | lyarwood: i think it will work | |
| 10:22:10 | lyarwood | okay something is still installing bandit in the grenade jobs as well on stable/train FWIW | |
| 10:22:16 | lyarwood | brb | |
| 10:22:36 | elod | the question is whether there are some benefit if we move linters to tox.ini instead. which I don't see yet, as that would add another place where we should look for dependencies... but maybe I'm wrong :X | |
| 10:24:33 | stephenfin | elod: Is it actually a dependency? You don't need it to run the main code nor the tests (for anything that doesn't have tests for custom linters, that is). It's a dependency but only in the same way tox is a dependency | |
| 10:25:13 | lyarwood | stephenfin: there's a bandit env in tox | |
| 10:25:30 | lyarwood | stephenfin: I assume that's why it's there? | |
| 10:25:47 | stephenfin | I don't get you. wdym? | |
| 10:26:05 | lyarwood | [testenv:bandit] | |
| 10:26:07 | lyarwood | # NOTE(browne): This is required for the integration test job of the bandit | |
| 10:26:09 | lyarwood | # project. Please do not remove. | |
| 10:26:11 | lyarwood | envdir = {toxworkdir}/shared | |
| 10:26:13 | lyarwood | commands = bandit -r nova -x tests -n 5 -ll | |
| 10:26:15 | lyarwood | ^ in tox.ini on stable/train | |
| 10:27:06 | stephenfin | oh, okay, I'm not saying we don't need to specify bandit somewhere. I'm saying we don't need to do it in test-requirements.txt because it doesn't need to be subject to e.g. lower-constraints checks | |
| 10:27:14 | stephenfin | we can do it in tox.ini instead | |
| 10:27:18 | lyarwood | ah right sorry | |
| 10:27:49 | lyarwood | yeah well I get elod's point that it's just another place to look for deps but if it isn't needed outside of that tox env then I'd be okay with just listing it there in tox.ini | |
| 10:28:11 | stephenfin | In case it helps, the way I was diagnosing those lower-constraints jobs yesterday was to create a new virtualenv, update pip and run the same command as the lower-constraints tox target | |
| 10:28:20 | lyarwood | do you want to push a change on master? | |
| 10:28:35 | stephenfin | I had to do that because on Fedora 33, I get Python 3.9 in my virtualenv which isn't compatible with a few of the dependencies | |
| 10:28:53 | lyarwood | yeah I just hacked the base python version when working on this the other day | |
| 10:29:08 | lyarwood | upgraded pip and reproduced the issue | |
| 10:29:14 | lyarwood | but that's different to this issue again | |
| 10:29:23 | stephenfin | Fair | |
| 10:29:24 | lyarwood | this was just an uncapped dep dropping py2 support | |
| 10:29:53 | lyarwood | tbh I think we might need to do that until py39 is actually supported | |
| 10:30:07 | lyarwood | otherwise some of us on modern distros get stung all the damn time | |
| 10:30:17 | stephenfin | yuuup | |
| 10:30:49 | stephenfin | fwiw, you can also do this | |
| 10:31:02 | lyarwood | oh cool there's a LC failure now on stable/train as well | |
| 10:31:03 | stephenfin | TOX_CONSTRAINTS_FILE=lower-constraints.txt tox -e py36 | |
| 10:31:22 | stephenfin | sub UPPER_ for TOX_ on pre-victoria iirc | |
| 10:31:31 | stephenfin | \o/ | |
| 10:31:41 | elod | wasn't there another discussion in mailing list that the usage of basepython is discouraged? o:) | |
| 10:32:03 | stephenfin | unless this was recently, I fixed that | |
| 10:32:16 | lyarwood | ERROR: No matching distribution found for hacking<1.2.0,>=1.1.0 | |
| 10:32:23 | lyarwood | ^ stephenfin was that the LC issue you were working on? | |
| 10:33:36 | elod | this 'no matching distro' issue seems more like some mirror thing to me. (and again, I might be wrong :X) | |
| 10:33:50 | stephenfin | Not that exact, but it looks familiar. That happens because it can't match the dependencies of that hacking version with those required by other dependencies | |
| 10:33:54 | stephenfin | It's very misleading | |
| 10:34:18 | lyarwood | yeah I thought that the other day but talking to fungi we found https://review.opendev.org/c/openstack/nova/+/765824 | |
| 10:34:42 | lyarwood | stephenfin: yeah indeed it's an awful error message | |
| 10:35:40 | elod | :S | |
| 10:38:48 | lpetrut | hi, I have a quick question about the lower-constraints file: it's supposed to contain only direct dependencies, right? for example, if we need package x, which in turn requires package y, would package y need to be in lower-constraints.txt? | |
| 10:39:12 | stephenfin | lpetrut: it would, yes | |
| 10:39:47 | stephenfin | However, we haven't been very good around managing that since the tooling situation is quite poor | |
| 10:40:09 | stephenfin | So I don't think anyone is going to hold it against you in a review | |
| 10:41:18 | lpetrut | stephenfin: thanks for clearing it out. yep, it's really difficult to maintain, I was hoping to be able to limit lower-constraints to direct dependencies | |
| 10:41:57 | lpetrut | but I guess that would affect its usefulness | |
| 10:43:44 | lyarwood | stephenfin: which version of py36 are you using btw? | |
| 10:44:52 | lyarwood | stephenfin: everything is borked for me with 3.6.12 with setuptools 49.1.3 /o\ | |
| 10:45:21 | lyarwood | and https://github.com/pypa/setuptools/issues/2017 | |
| 10:45:55 | stephenfin | I was using 3.6.12, but that was with OSC, not nova | |