| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-12-09 | |||
| 08:06:00 | openstackgerrit | Mamduh proposed openstack/os-vif stable/ussuri: Refactor code of linux_net to more cleaner and increase performace https://review.opendev.org/c/openstack/os-vif/+/765419 | |
| 08:12:42 | gibi | LarsErikP: sure, it can be backported. But please propose a backport to stable/victoria first then stable/ussuri | |
| 08:15:46 | LarsErikP | melwitt: Uh.. Don't know what happended there. Forgot the link: https://review.opendev.org/c/openstack/nova/+/759348/ | |
| 08:16:51 | LarsErikP | gibi: Uhm.. not sure howto do that. Maybe melwitt could do it, as she fixed this originally? =) | |
| 08:18:11 | openstackgerrit | Mamduh proposed openstack/os-vif stable/ussuri: Fix - os-vif fails to get the correct UpLink Representor https://review.opendev.org/c/openstack/os-vif/+/765967 | |
| 08:20:30 | openstackgerrit | Lars Erik Pedersen proposed openstack/nova stable/victoria: Omit resource inventories from placement update if zero https://review.opendev.org/c/openstack/nova/+/766177 | |
| 08:20:53 | LarsErikP | gibi: I found the button ^ :P | |
| 08:23:13 | LarsErikP | should I add reviewers? and.. there is merge conflicts in ussuri. not feeling comfortable to deal with that :S | |
| 08:25:53 | openstackgerrit | Mamduh proposed openstack/os-vif stable/train: Refactor code of linux_net to more cleaner and increase performace https://review.opendev.org/c/openstack/os-vif/+/765912 | |
| 08:30:29 | gibi | LarsErikP:added some stable cores to the review. Thanks for proposing the backport | |
| 08:31:37 | gibi | LarsErikP: as for the ussuri backport, lets merge the victoria one first then I think melwitt can resolv the merge conflict in ussuri or I can take it if needed | |
| 08:32:13 | LarsErikP | gibi: good plan :-) thanks so much! | |
| 08:32:37 | gibi | LarsErikP: :) | |
| 08:44:37 | bauzas | good morning Nova | |
| 08:55:18 | gibi | bauzas: O/ | |
| 09:30:54 | openstackgerrit | Adrian Chiris proposed openstack/os-vif stable/ussuri: Fix - os-vif fails to get the correct UpLink Representor https://review.opendev.org/c/openstack/os-vif/+/765967 | |
| 09:46:15 | lyarwood | elod: morning, have you had anytime to look at the pip failures in stable before I start digging in? | |
| 09:47:59 | lyarwood | elod: https://review.opendev.org/q/Ia2007bc63ef09931ea0197cef29d6a5614ed821a - I was checking in on this series and noticed that everything prior to victoria is failing with pip 20.2.4 | |
| 09:48:02 | elod | lyarwood: good morning :) I'm looking at several issues now, which pip failures do you mean? :) | |
| 09:48:13 | lyarwood | elod: https://zuul.opendev.org/t/openstack/build/cb6247d4b3644045ab6d83a064e812c6 for example | |
| 09:48:33 | lyarwood | actually ussuri looks okay sorry | |
| 09:49:07 | lyarwood | ERROR: Package 'bandit' requires a different Python: 2.7.17 not in '>=3.5' | |
| 09:49:10 | elod | yes, this bandit is what I just started to look at | |
| 09:49:35 | elod | I guess we've got a new bandit version with incorrect setup.cfg :S | |
| 09:49:51 | elod | but haven't checked yet | |
| 09:51:04 | elod | at least there was a release, for sure: https://pypi.org/project/bandit/#history | |
| 09:51:14 | lyarwood | oh it's just a new version that drops py2 support | |
| 09:51:35 | lyarwood | okay we can cap this easily on stable where we are still using py2 | |
| 09:53:10 | elod | I'll look for the bandit changes and let's see if I can put this new version to disallow-list in global requirements | |
| 09:54:25 | lyarwood | https://github.com/PyCQA/bandit/releases/tag/1.6.3 it dropped py2 support | |
| 09:55:30 | lyarwood | elod: https://review.opendev.org/c/openstack/requirements/+/766170 | |
| 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 | |