| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-01-09 | |||
| 10:40:18 | gibi__ | stephenfin: I tried in tox 3.28 and I did not need ingnore_base_python_conflict to make it work | |
| 10:40:44 | gibi__ | stephenfin: you can simply remova that from nova's tox.ini and it work in tox 3.28 for me locally | |
| 10:42:42 | gibi__ | stephenfin: anyhow so you suggest to just remove basepython = python3 and assume people's machine has python3 by default | |
| 10:43:03 | gibi__ | (which could be a fair assumption having python2.7 in EOL) | |
| 10:43:32 | stephenfin | I think that's a reasonable workaround, yes | |
| 10:44:30 | gibi__ | OK, I will change https://review.opendev.org/c/openstack/nova/+/869545 to drop basepython instead of defining it for each generative env | |
| 10:45:15 | gibi__ | and then I can also try to drop ignore_basepython_conflict | |
| 10:45:44 | stephenfin | oh, you can do the two in one go. ignore_basepython_conflict is only needed if `basepython` is defined (which it won't be here) | |
| 10:46:01 | gibi__ | OK | |
| 10:50:28 | sean-k-mooney | bauzas: yes you can use tox -e py3|functional | |
| 10:50:42 | sean-k-mooney | those will use your defalt python | |
| 10:51:20 | sean-k-mooney | we only set basepython to cater for python2 vs python3 | |
| 10:51:26 | sean-k-mooney | so yes we can drop it now | |
| 10:53:47 | opendevreview | Balazs Gibizer proposed openstack/nova master: Remove basepython def from tox.ini https://review.opendev.org/c/openstack/nova/+/869545 | |
| 10:53:54 | gibi__ | stephenfin, bauzas: ^^ | |
| 10:54:04 | sean-k-mooney | gibi__: so regarding https://review.opendev.org/c/openstack/os-vif/+/869500 that now works but we need to squash it into your patch | |
| 10:54:23 | sean-k-mooney | or in your patch you can disable the functional jobs and we can re enable it in this one | |
| 10:54:29 | sean-k-mooney | gibi__: any prefernce | |
| 10:56:36 | gibi__ | I need to update that tox patch to remove basepython and ignore_basepython_conflict, I can do a squash at the same time | |
| 10:57:00 | darkhorse | artom: If you remember our discussion on unshelving pci instance, I tried to boot from image that is related to the unshelved instance but failed. I don't see an image created when I shelve an instance. I tried openstack images list and also checked in the glace>images table but nothing is created when I shelve an instance. | |
| 10:57:41 | gibi__ | sean-k-mooney: but I need to have lunch first | |
| 10:59:13 | sean-k-mooney | gibi__: cool works for me enjoy your lunch | |
| 11:00:43 | gibi__ | thanks | |
| 11:31:40 | opendevreview | Balazs Gibizer proposed openstack/os-vif master: Make tox.ini tox 4.0.0 compatible https://review.opendev.org/c/openstack/os-vif/+/868420 | |
| 11:31:51 | gibi__ | sean-k-mooney: ^^ fix with the squash | |
| 11:40:56 | sean-k-mooney | gibi__: thanks +2 although ignoring 2 +2s thing for a sec given i wrote part of this i want someone else to +w anyway | |
| 11:42:51 | opendevreview | sean mooney proposed openstack/os-vif master: Update gate jobs as per the 2023.1 cycle testing runtime https://review.opendev.org/c/openstack/os-vif/+/861468 | |
| 11:51:35 | gibi__ | stephenfin: if you have a sec then here https://review.opendev.org/c/openstack/placement/+/868418 I think we have some disagreements | |
| 11:56:32 | sean-k-mooney | gibi__: the convention we had for the deps is generally to inherit and extend | |
| 11:57:00 | sean-k-mooney | like this | |
| 11:57:01 | sean-k-mooney | deps = | |
| 11:57:03 | sean-k-mooney | {[testenv]deps} | |
| 11:57:05 | sean-k-mooney | -r{toxinidir}/doc/requirements.txt | |
| 11:57:16 | sean-k-mooney | i think that is what stephen was sugessing but not sure | |
| 11:57:37 | sean-k-mooney | to avoid repeatign | |
| 11:57:39 | sean-k-mooney | -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} | |
| 11:57:41 | sean-k-mooney | -r{toxinidir}/requirements.txt | |
| 12:01:06 | gibi__ | sean-k-mooney: extending deps alone is not enough, it seems deps are not properly applied during the editable install of the package | |
| 12:01:28 | gibi__ | basically the editable install makes the deps unconstrained | |
| 12:01:40 | gibi__ | (from above) | |
| 12:01:55 | sean-k-mooney | really thats not what i was expecting | |
| 12:02:12 | sean-k-mooney | i guess that is a delete in the skipsdist behavior | |
| 12:02:24 | sean-k-mooney | by not skiping we build a wheel and install form that | |
| 12:02:49 | stephenfin | gibi__: looking | |
| 12:03:01 | sean-k-mooney | presumable its the build step that might be the issue? | |
| 12:04:15 | stephenfin | oh, interesting | |
| 12:06:13 | gibi__ | sean-k-mooney: yes it could be related to removing skipdist | |
| 12:06:19 | sean-k-mooney | use_develop with skipsdist=false seams to not be doing an editiable install it instead ensure that a install of the current content of the working dir is done via a wheel i think | |
| 12:06:37 | stephenfin | hmm, so in theory, either way should be okay. If we're separately listing 'requirements.txt' in our 'deps' setting then we'll have installed those (with constraints) first. That's because we've removed skipsdist alright | |
| 12:07:26 | sean-k-mooney | we coudl just move the -c line to the deps of testenv | |
| 12:07:34 | sean-k-mooney | then do {[testenv]deps} | |
| 12:07:36 | stephenfin | But yeah, overriding 'install_command' is certainly less likely to cause issue | |
| 12:07:42 | sean-k-mooney | anytime we are modifing the deps later | |
| 12:08:04 | stephenfin | sean-k-mooney: what gibi's saying though is that the package itself (i.e. placement or nova) is installed without constraints | |
| 12:08:34 | stephenfin | so we're relying on us having already installed the deps with constraints and hoping (I guess) that tox doesn't decide to reinstall them without constraints when installing the package | |
| 12:08:36 | gibi__ | stephenfin: we install our deps with constraints yes, but then we install the placement package's deps again without constriants | |
| 12:08:54 | stephenfin | so best to override the install_command | |
| 12:09:02 | sean-k-mooney | ok but woudl we not | |
| 12:09:13 | sean-k-mooney | ya would we not need to change that ^ | |
| 12:09:22 | stephenfin | we should drop '-rrequirements.txt' from 'deps' too since it's now unnecessary and potentially confusing | |
| 12:09:51 | gibi__ | OK, I can move everything to install_command | |
| 12:09:51 | sean-k-mooney | this looks a littel differnt then normal actully | |
| 12:10:00 | gibi__ | and drop deps | |
| 12:10:36 | sean-k-mooney | https://github.com/openstack/os-vif/blob/master/tox.ini | |
| 12:10:38 | stephenfin | I'd keep 'deps' but only for test-requirements.txt and doc/requirements.txt | |
| 12:10:49 | sean-k-mooney | if we compare it to osvif we droped the install_command a while ago | |
| 12:11:09 | stephenfin | sean-k-mooney: you've got 'skipsdist = true' though | |
| 12:11:23 | sean-k-mooney | true | |
| 12:11:25 | stephenfin | so it won't actually install os-vif | |
| 12:11:30 | sean-k-mooney | but for nova we doint | |
| 12:11:36 | sean-k-mooney | https://github.com/openstack/nova/blob/master/tox.ini | |
| 12:11:40 | stephenfin | Yup, so it will install nova | |
| 12:12:20 | sean-k-mooney | ok that is why the os-vif packages were not being installed | |
| 12:12:20 | stephenfin | os-vif is working because Python adds $PWD to $PYTHONPATH | |
| 12:12:29 | stephenfin | or rather, was | |
| 12:12:39 | stephenfin | but I think tox 4 might have changed that, idk | |
| 12:12:43 | sean-k-mooney | we could have actully kept the two commits sepeate instaed fo squashihng them | |
| 12:13:27 | sean-k-mooney | stephenfin: i had to rebase this on top of gibis tox4 change https://review.opendev.org/c/openstack/os-vif/+/869500/2 because the os-vif package was not found | |
| 12:13:56 | sean-k-mooney | this combined one works https://review.opendev.org/c/openstack/os-vif/+/868420/3 | |
| 12:14:09 | sean-k-mooney | but we also could have used install command i guess | |
| 12:15:57 | stephenfin | gibi__: one other thing: instead of 'pip install' can we do 'python -m pip install' | |
| 12:16:17 | stephenfin | there's a reason for that that clarkb brought up on openstack-discuss recently (I think) but I don't recall what | |
| 12:16:24 | stephenfin | it's what tox itself does though | |
| 12:17:07 | sean-k-mooney | i have been pushing people to move to -m pip install and got that changed in devstack | |
| 12:17:43 | sean-k-mooney | there are some odd behaivors with user install/venvs when using pip that does not happen if you use python -m pip | |
| 12:18:12 | sean-k-mooney | bascailly you can have the pip package isntalled without the pip script being in your current path | |
| 12:19:10 | sean-k-mooney | also due to how setup tools console scripts work there can also be some funkyness in a mixed python2/python3 enve with the #! line | |
| 12:19:27 | sean-k-mooney | so the module apporch is just generally more robost | |
| 12:23:34 | gibi__ | ack | |
| 13:46:03 | opendevreview | Balazs Gibizer proposed openstack/placement master: Make tox.ini tox 4.0.0 compatible https://review.opendev.org/c/openstack/placement/+/868418 | |
| 13:46:43 | gibi | stephenfin, sean-k-mooney: ^^ | |
| 13:47:45 | kashyap | gibi: What do you think of workarounds like this? -- https://review.opendev.org/c/openstack/nova/+/869536/ (I'm not a fan of it...) | |
| 13:47:50 | kashyap | (Gibi or anyone :-)) | |
| 13:52:04 | kashyap | (You can comment directly on the review.) | |
| 13:54:06 | gibi | sean-k-mooney, stephenfin: I cannot switch os-vif to install_command from deps it results in https://paste.opendev.org/show/bkSH8jaL4WH04S2sge5t/ | |
| 13:54:47 | gibi | basically we have a constraing about os-vif in global requirements so the editable install building locally will conflict with the global constraint | |
| 13:55:11 | stephenfin | okay, I guess we need to continue using 'deps' there so | |
| 13:55:35 | gibi | stephenfin: even if it means we allow ignoring the upper constraint during the editable install? | |
| 13:55:52 | stephenfin | we'll still install the deps first, right? | |
| 13:58:01 | gibi | right | |