| opendevreview | Eric Xie proposed openstack/os-vif master: Fix typos https://review.opendev.org/c/openstack/os-vif/+/810137 | 01:34 |
|---|---|---|
| opendevreview | Balazs Gibizer proposed openstack/nova stable/xena: [stable-only]Update .gitreview for stable/xena https://review.opendev.org/c/openstack/nova/+/809759 | 07:15 |
| opendevreview | Balazs Gibizer proposed openstack/nova stable/xena: [stable-only]Update TOX_CONSTRAINTS_FILE for stable/xena https://review.opendev.org/c/openstack/nova/+/809760 | 07:16 |
| opendevreview | Balazs Gibizer proposed openstack/nova stable/xena: [stable-only]Update TOX_CONSTRAINTS_FILE for stable/xena https://review.opendev.org/c/openstack/nova/+/809760 | 07:16 |
| gibi | zigo, artom, clarkb: I've updated the stable/xena setup patches | 07:17 |
| gibi | I think in general the backport check helps enforceing that we only merge patches in a good branch order | 07:17 |
| gibi | new stable branch setup seems to be an edge case | 07:18 |
| bauzas | good morning folks | 07:19 |
| bauzas | gibi: hmmmm | 07:20 |
| gibi | bauzas: good morning | 07:21 |
| bauzas | gibi: modifying the commit msg would mean that the CI job would accept it ? | 07:21 |
| bauzas | haha | 07:21 |
| bauzas | "Stable branch requires either cherry-pick -x headers or [stable-only] tag!" | 07:21 |
| gibi | bauzas: yes, either the patch needs to have a cherry-picked from line where the hash points to a merged change or the patch needs to state that it is stable only | 07:22 |
| bauzas | saw in https://zuul.opendev.org/t/openstack/build/97c7712edfe34c5386c0cc2fc37ef466 | 07:22 |
| bauzas | gibi: kk, +2 then | 07:22 |
| *** rpittau|afk is now known as rpittau | 07:24 | |
| bauzas | gibi: looks like we need to do this also for all of the changes https://review.opendev.org/q/owner:infra-root%2540openstack.org+(project:openstack/nova+OR+project:openstack/placement)+is:open | 07:24 |
| bauzas | ah no | 07:28 |
| bauzas | just rechecks because of timeouts in placement | 07:29 |
| gibi | I think we dont have the backport check in placement | 07:34 |
| gibi | the placement lower constraint timeout needs https://review.opendev.org/c/openstack/placement/+/810001 to merge first | 07:35 |
| bauzas | gibi: this is correct, hence my "ah no" :) | 07:48 |
| bauzas | (placement doesn't verify the backports) | 07:48 |
| bauzas | gibi: and shit, you're right | 07:49 |
| bauzas | we need a second core | 07:49 |
| bauzas | lyarwood: I know your placement knowledge is the same than old greek, but we'd appreciate a +W for a dependency bump https://review.opendev.org/c/openstack/placement/+/810001 in order to fix the gate | 07:51 |
| gibi | bauzas: also the stable team needs to make a short term decision what to do with the lower constraint bump on stable/xean | 07:56 |
| bauzas | we have an open change about it | 07:56 |
| gibi | it is not really allowed, but we can push an RC2 to be in the xena release | 07:56 |
| bauzas | https://review.opendev.org/c/openstack/placement/+/787863 | 07:57 |
| gibi | yeah that also possible | 07:57 |
| gibi | turning off the job | 07:57 |
| bauzas | honestly, I have no opinion and this could be discussed at the PTG | 07:57 |
| bauzas | but we need to consider the bump, agreed | 07:58 |
| gibi | so 1) bump lower constarint and release RC2 so we are not breaking stable policy abour reqs 2) try to pin virtualenv during tox installation to avoid unpinned setuptools on stable (this is the long term solution) 3) turn off the lower job | 07:58 |
| gibi | and the problem is not placement specific but hitting a lot of projects | 07:59 |
| gibi | nova is not impacted | 07:59 |
| gibi | so a common solution would be good | 07:59 |
| bauzas | gibi: let's discuss this during our meeting tonight | 08:20 |
| gibi | sure | 08:20 |
| gibi | bauzas: btw would you like to take over charing of the meeting? | 08:20 |
| bauzas | gibi: I was thinking about it | 08:20 |
| bauzas | I can do it :) | 08:20 |
| gibi | elodilles: will you be available on todays meeting to talk about the stable req bump problem? | 08:20 |
| gibi | bauzas: then I officially give you the baton :) | 08:21 |
| bauzas | huuuuuuuuuuh | 08:21 |
| bauzas | https://www.youtube.com/watch?v=43RID9cIEAE | 08:23 |
| bauzas | DOOOOOH | 08:23 |
| gibi | bauzas: https://review.opendev.org/c/opendev/irc-meetings/+/810165 | 08:24 |
| bauzas | gibi: thanks, was looking on it...after finding the Simpson video :) | 08:25 |
| * bauzas should start modifying a few things https://docs.openstack.org/nova/latest/contributor/ptl-guide.html#new-ptl | 08:26 | |
| gibi | bauzas: on the today meeting we should also talk about the release liaison role | 08:26 |
| bauzas | gibi: yeah, I was about to ask for it | 08:27 |
| gibi | cool | 08:27 |
| bauzas | also, we have other liaisons fwiw | 08:27 |
| gibi | for me only the release one was visible | 08:28 |
| bauzas | technically we have other roles in our team :) https://wiki.openstack.org/wiki/Nova#People | 08:28 |
| bauzas | but meh | 08:28 |
| bauzas | it's not like we have 50 engineers wondering how to help our community :) | 08:29 |
| lyarwood | \o mornig | 08:31 |
| * lyarwood reads | 08:31 | |
| elodilles | gibi: yes, i'll be there on the meeting :) | 08:35 |
| gibi | elodilles: do you already see a common solution forming from stable perspective? | 08:38 |
| gibi | lyarwood: o/ | 08:42 |
| elodilles | o/ | 08:45 |
| elodilles | gibi: btw, I agree with your option 2 but needs to be checked with infra I think | 08:46 |
| gibi | elodilles: yeah, I can accept that option 2 needs more effort probaly to make it right so if we need a short term solution then I'm fine with either 1 or 3. 1 has the limit that we need to cut RC this week. 3 has the effect that we will never turn the job back again | 08:48 |
| gibi | hm feels like the gate is clogged | 08:50 |
| gibi | :/ | 08:50 |
| bauzas | gibi: elodilles: fwiw, I'm in favor of bumping the dep now, which is option 1 and revisit the issue at the PTG | 09:14 |
| bauzas | short-term, we need to addree the fire | 09:15 |
| bauzas | address* | 09:15 |
| bauzas | long-term, we could draft a plan on how to avoid fire next time | 09:15 |
| gibi | bauzas: I was cheated a bit with 1). on stable/xena this is viable as we can have RC2. But on older stable branches we only have the option of 2) or 3) (or going against the stable req policy) | 09:28 |
| bauzas | gibi: what policy issue do you see ? we can bump a .y release which can need to upgrade a dependency, right? | 09:37 |
| alexe9191 | good day everyone :) after restarting the scheduler after an OpenStack upgrades all of the schedulers are running 100% cpu and are taking a lot of time building the cache. Scheduling is not working of course as the schedulers are busy creating this cache. | 09:41 |
| alexe9191 | I am wondering if adding more schedulers would solve this problem ? current resources allocated to each scheduler is about 8G of memory and 8 CPUS | 09:41 |
| alexe9191 | but I do not think that the # of cpu matters as it seems that the python process is single threaded. I also did not find any "workers" configuration for the scheduler in the newton version. | 09:42 |
| gibi | bauzas: on stable we should not bump major version of a dependency I think | 09:44 |
| bauzas | for a .z release, yes | 09:44 |
| gibi | can we do that for a .y. release? | 09:44 |
| bauzas | lemme try to look at the Openstack semver rules | 09:45 |
| bauzas | that's not really a "stable policy" AFAIK | 09:45 |
| bauzas | at least, I can't find anything in https://docs.openstack.org/project-team-guide/stable-branches.html | 09:45 |
| bauzas | there it is https://docs.openstack.org/pbr/latest/user/semver.html#semantic-versioning-specification-semver | 09:46 |
| bauzas | alas, can't find anything specific | 09:48 |
| bauzas | we won't technically change placement API if we bump the minimum | 09:49 |
| bauzas | gibi: I guess we should raise this question to the wider community, maybe | 09:49 |
| bauzas | and see whether they freak out about upgrading our deps in a .y release | 09:50 |
| alexe9191 | anyone on the scheduling issue :) ? | 09:50 |
| opendevreview | Merged openstack/placement master: Bump min decorator to 4.0.0 https://review.opendev.org/c/openstack/placement/+/810001 | 09:54 |
| bauzas | alexe9191: we superseded the idea to have multiple workers by telling we should rather use Placement | 09:54 |
| bauzas | alexe9191: for the CPU usage, try to get a GMR | 09:54 |
| bauzas | alexe9191: https://docs.openstack.org/nova/latest/reference/gmr.html | 09:55 |
| bauzas | so you should see the greenlets and greenthreads concurrently running | 09:55 |
| alexe9191 | I am still on the way to upgrade to rocky and to use the placement fully. I can not drop the sceduler in newton as far as I know | 09:56 |
| bauzas | nova-scheduler will continue to exist | 09:56 |
| bauzas | it's just that scheduler will call out placement for getting a list of candidates before running | 09:56 |
| bauzas | which reduces the amount of complexity by adding a single-lock mechanism | 09:57 |
| alexe9191 | We will start using the placement UI as recommended. However we need to upgrade first. We are coming from Kilo and need to transit first at newton before we continue the migrations. | 09:58 |
| alexe9191 | The scheduler is running at 100% cpu for about an hour now and updating host stats/aggregates etc as I can see in the log files. And as I understand, this needs to be done first before the scheduler starts scheduling anything by either using placement or using the filters. | 09:59 |
| alexe9191 | So my question is, would adding more workers speed up the process of building that cache? or it's just a matter of waiting | 09:59 |
| gibi | bauzas: good idea to start a discussion around this in the wider community | 10:02 |
| bauzas | alexe9191: honestly, updating the cache shouldn't take one hour | 10:14 |
| bauzas | you have another issue | 10:14 |
| alexe9191 | What could that be? | 10:15 |
| bauzas | I dunno, get the GMR | 10:15 |
| alexe9191 | I see a ton of those messages `Update host state with aggregates` | 10:15 |
| bauzas | how many aggregates do you have ? | 10:15 |
| alexe9191 | We also have about 900 hosts and more than 10K vms | 10:15 |
| alexe9191 | one moment | 10:16 |
| alexe9191 | 9 | 10:16 |
| alexe9191 | I also see a lot of those "Update host state with service dict" | 10:16 |
| bauzas | 9 can't be an issue | 10:22 |
| bauzas | again, please do a GMR and see what threads run | 10:23 |
| alexe9191 | I am honestly not sure what you mean when you say do a GMR? :) | 10:23 |
| alexe9191 | ah `kill -USR2 8675 ` | 10:24 |
| alexe9191 | that wouldn't terminate the process right ? | 10:24 |
| alexe9191 | Nova did not generate any report | 10:37 |
| sean-k-mooney | alexe9191: it will more or less kille the process | 10:42 |
| sean-k-mooney | the report is stored in the nova log | 10:42 |
| sean-k-mooney | where ever you redirect that too | 10:42 |
| sean-k-mooney | to fix the process you will need to restart it | 10:42 |
| alexe9191 | I did send the USR2 signal but the logs where empty | 10:43 |
| alexe9191 | well not empty I did not see any GMR logs | 10:44 |
| sean-k-mooney | what process did you send it to | 10:44 |
| alexe9191 | nova-scheduler | 10:44 |
| alexe9191 | and also nova-api as a test | 10:44 |
| sean-k-mooney | nova-api wont work if its running under appache or uwsgi | 10:44 |
| alexe9191 | I am actually wondering if having debug enabled is causing an overhead? | 10:44 |
| sean-k-mooney | but the shcheduler should | 10:44 |
| sean-k-mooney | debug logging | 10:44 |
| alexe9191 | it's not it's running under dumb-init. All of the processes are running in containers. | 10:44 |
| sean-k-mooney | am if you have slow disks then it could | 10:45 |
| sean-k-mooney | oh this is kolla | 10:45 |
| alexe9191 | Yeah more or less, kolla containers | 10:45 |
| sean-k-mooney | so dumb-init is likely not passing the signal | 10:45 |
| alexe9191 | hmmmm | 10:45 |
| sean-k-mooney | that said if you used the pid of the python process not dumb-init it should work | 10:45 |
| alexe9191 | Let me try it out again | 10:46 |
| alexe9191 | Nothing | 10:50 |
| alexe9191 | I do however see a lot of those in the logs "Lock "host_instance" acquired by "nova.scheduler.host_manager.sync_instance_info"" | 10:50 |
| opendevreview | Vlad Gusev proposed openstack/nova stable/stein: Add regression test for bug #1908075 https://review.opendev.org/c/openstack/nova/+/810191 | 10:51 |
| alexe9191 | and now after about 90 minutes it's done | 11:13 |
| gibi | bauzas: could you please triage this vgpu bug https://bugs.launchpad.net/nova/+bug/1943933 ? | 11:30 |
| sean-k-mooney | i think the way our config is ment to work is we list the adress of the parent | 11:33 |
| sean-k-mooney | and then report the quantity of the mdev type that can be created for that device | 11:33 |
| gibi | ohh so the reported wants to partially report the possible mdevs from a physical device | 11:34 |
| sean-k-mooney | i think so | 11:34 |
| sean-k-mooney | which i dont think we support | 11:34 |
| sean-k-mooney | they basically want to treat it like we do for sriov i think where you can enable indiviugual vfs | 11:34 |
| gibi | so this would be a new feature | 11:35 |
| sean-k-mooney | i could be reading into the report too much but i think that is what they were expecting | 11:35 |
| sean-k-mooney | gibi: ya basically a host_reserved_mdev parmater or somehting | 11:35 |
| gibi | OK. I let bauzas respond but I think I see what you see in the report | 11:36 |
| sean-k-mooney | e.g. enable all mdevs of this type form this parent but reserve x for host use | 11:36 |
| gibi | bauzas: also ther is another vgpu bug to triage https://bugs.launchpad.net/nova/+bug/1944031 | 11:36 |
| sean-k-mooney | the second one likely is correct but never personally used vgpu so not sure | 11:37 |
| gibi | yeah bauzas has an environment to reproduce :) | 11:37 |
| gibi | hence my ping | 11:37 |
| bauzas | gibi: sean-k-mooney: ack, will look at both | 12:03 |
| gibi | bauzas: thanks | 12:04 |
| gibi | sean-k-mooney: this feels like a networking / neutron bug but I'm not certain. cloud you check it please https://bugs.launchpad.net/nova/+bug/1944083 ? | 12:17 |
| sean-k-mooney | gibi: the only /32 that i can think of that we install is the one for the metadata service | 12:27 |
| sean-k-mooney | gibi: but ya i think this is all contoled more or less on the neutron side in combindation with cloud-init | 12:28 |
| sean-k-mooney | we might store some to the network info in the metadata which will be used by cloud-init | 12:28 |
| sean-k-mooney | but i think this is more or less out of our contol | 12:29 |
| sean-k-mooney | lets add neutron and see what they think | 12:30 |
| gibi | sean-k-mooney: thanks for the analysis | 12:31 |
| gibi | I agree to involve neutron | 12:31 |
| sean-k-mooney | the closet thing i can tink of is https://github.com/openstack/nova/blob/master/nova/virt/interfaces.template | 12:32 |
| sean-k-mooney | we do list the dhcp server per interface https://github.com/openstack/nova/blob/master/nova/virt/interfaces.template#L21 | 12:33 |
| sean-k-mooney | but we are not adding any routes here | 12:33 |
| sean-k-mooney | we do set the default route via the gateway in the metadata https://github.com/openstack/nova/blob/50fdbc752a9ca9c31488140ef2997ed59d861a41/nova/virt/netutils.py#L326-L348 | 12:39 |
| sean-k-mooney | and populate any addtional neutron routes https://github.com/openstack/nova/blob/50fdbc752a9ca9c31488140ef2997ed59d861a41/nova/virt/netutils.py#L103-L121 | 12:39 |
| sean-k-mooney | but ya as far as i can see there is noting on the nova side that would install /32 routes to the dns servers | 12:40 |
| opendevreview | norman shen proposed openstack/nova master: Recreate mdev devices according to placement https://review.opendev.org/c/openstack/nova/+/810220 | 12:40 |
| opendevreview | Merged openstack/nova master: Add missing __init__.py in nova/db/api https://review.opendev.org/c/openstack/nova/+/809980 | 12:54 |
| opendevreview | Merged openstack/nova stable/xena: [stable-only]Update .gitreview for stable/xena https://review.opendev.org/c/openstack/nova/+/809759 | 12:54 |
| opendevreview | Merged openstack/nova stable/xena: [stable-only]Update TOX_CONSTRAINTS_FILE for stable/xena https://review.opendev.org/c/openstack/nova/+/809760 | 12:54 |
| opendevreview | Balazs Gibizer proposed openstack/nova stable/xena: Add missing __init__.py in nova/db/api https://review.opendev.org/c/openstack/nova/+/810192 | 13:14 |
| gibi | elodilles, lyarwood, bauzas: we need this in xena for RC2 ^^ | 13:15 |
| bauzas | done | 13:16 |
| gibi | thanks | 13:16 |
| lyarwood | ACK'd | 13:16 |
| gibi | cool | 13:17 |
| gibi | this was fast :) | 13:17 |
| lyarwood | I'm going to be AFK for the meeting btw, dental checkup for the first time in a long time. | 13:17 |
| gibi | lyarwood: ack, thanks for the headsup | 13:18 |
| gibi | and I hope the dentis will be painless | 13:18 |
| opendevreview | Lee Yarwood proposed openstack/nova-specs master: Store and allow libvirt instance device buses and models to be updated https://review.opendev.org/c/openstack/nova-specs/+/810235 | 13:21 |
| lyarwood | I'm British so the normal and in my case correct stereotypes apply, it's not going to be fun. | 13:22 |
| bauzas | lyarwood: we need a release liaison that I'll ask in today's meeting | 13:24 |
| bauzas | lyarwood: if you are interested in this role, tell me | 13:24 |
| gibi | this reminds me to book a dentis checkup too | 13:24 |
| bauzas | gibi: booked since 6 months | 13:25 |
| gibi | I'm not rushing :) | 13:25 |
| kashyap | gibi: Yikes; I've been avoiding it | 13:25 |
| * bauzas loves doctor's appointments in my area, especially eye specialists and dentists | 13:25 | |
| kashyap | I know I should | 13:25 |
| bauzas | "I have a tooth rage", "sure, we have a slot in Feb 2022, works for you ?" | 13:26 |
| bauzas | longer than delivering a car | 13:26 |
| gibi | as far as I see I can book a dentist slot for this thursday morning | 13:36 |
| gibi | that feels too close :/ | 13:37 |
| kashyap | gibi: Don't you prefer your band-aid to be quickly removed? | 13:40 |
| gibi | that one way to look at it | 13:40 |
| artom_ | bauzas, obviously the solution is to drive a new car over your tooth | 13:41 |
| artom_ | No tooth, no pain | 13:41 |
| *** artom_ is now known as artom | 13:41 | |
| bauzas | heh | 13:41 |
| gibi | artom: :D | 13:42 |
| *** abhishekk is now known as akekane|home | 13:45 | |
| *** akekane|home is now known as abhishekk | 13:45 | |
| bauzas | gibi: agenda is updated https://wiki.openstack.org/wiki/Meetings/Nova#Agenda_for_next_meeting feel free to add your points | 13:52 |
| bauzas | gibi: will you propose a backport to stable/xena for the decorator bump ? | 13:52 |
| gibi | bauzas: ack will check the agenda | 13:52 |
| gibi | bauzas: I can propose a backport then we can drop it if we choose other option to go forward | 13:53 |
| opendevreview | Balazs Gibizer proposed openstack/placement stable/xena: Bump min decorator to 4.0.0 https://review.opendev.org/c/openstack/placement/+/810193 | 13:56 |
| gibi | bauzas: done | 13:56 |
| bauzas | thanks | 13:56 |
| bauzas | we could discuss this during the meeting then | 13:56 |
| gibi | added two topic to the end of the agenda | 14:02 |
| opendevreview | Thomas Goirand proposed openstack/nova stable/xena: Add missing __init__.py in nova/db/api https://review.opendev.org/c/openstack/nova/+/810192 | 14:20 |
| zigo | gibi: bauzas: My bad, I shouldn't have pushed an update, now this patch needs another +2W: https://review.opendev.org/c/openstack/nova/+/810192 | 14:21 |
| zigo | I was waiting for the .gitreview, and thought I should be doing it... | 14:21 |
| opendevreview | Elod Illes proposed openstack/nova stable/xena: Add missing __init__.py in nova/db/api https://review.opendev.org/c/openstack/nova/+/810192 | 14:33 |
| *** akekane_ is now known as abhishekk | 14:34 | |
| elodilles | zigo: i've uploaded the original patch again | 14:34 |
| zigo | elodilles: Thanks. | 14:35 |
| elodilles | zigo: and +2+W'd as the content is now the very same as PS1 had | 14:39 |
| clarkb | gibi: thats kind of my point though updating things like tox.ini or .gitreview or translations or other mechanical infrastructure bits don't need that and aren't really a corner case. The lint rules are just too aggressive and you can trust reviewers more imo | 14:48 |
| clarkb | It is ok to update the branch so that it functions | 14:48 |
| clarkb | but if you insist on having those rules please update the bots so that they don't run afoul of the linter | 14:49 |
| gibi | clarkb: I'm not the right person to convince as I wasnt the one pushed for the stable backport linter. lyarwood, elodilles: what is your view? | 14:52 |
| gibi | clarkb: I do think that having couple of paches needing a manual touch every six months is not a big deal. But I also see the point that we could update the bot to use a specific commit message in nova stable setup patches | 14:53 |
| clarkb | I probably feel more strongly about this than most because 90% of the time if I'm pushing to stable branches it is to fix a stable branch specific issue without a backport and I always run into problems like this and it is frustrating that no one seems to accept it is ok to fix a branch on its own | 14:53 |
| bauzas | gibi: oh shit, I wanted to discuss about the release liaison role but I forgot to add it in the meeting notes :facepalm: | 14:54 |
| gibi | bauzas: I added it :D | 14:54 |
| bauzas | I sqaw | 14:54 |
| clarkb | if the branch is broken and someone pushes a fix reviewers should have the discretion to land the fix | 14:54 |
| gibi | clarkb: thank you for taking case of the stable branches. I hope elodilles and lyarwood can join. | 14:55 |
| gibi | ... and add their views | 14:55 |
| gibi | s/case/care/ | 14:57 |
| gibi | clarkb: most of the linters are to help the reviews not missing things. in this case this linter helps the reviewer to check that the fix is already landed on master and newer stable branches. | 14:58 |
| bauzas | honeslty I tend to prefer trusting reviews rather than using linters | 14:58 |
| gibi | which is the rule, with exceptions | 14:58 |
| bauzas | but I think I already said that a lot (context: mypy) | 14:58 |
| elodilles | well, the point with that extra check is to ensure we have the *correct* patch backported | 14:59 |
| bauzas | if linters block us instead of helping us, then it's a problem | 14:59 |
| elodilles | as it sometimes happened that a patch was backported to several branch at the same time and later on modified (a middle patch) and those changes were not backported to older branches | 15:00 |
| gibi | bauzas: what if linters both help us but sometimes blocking us | 15:00 |
| elodilles | bauzas: don't look at that linter as a blocker but as a helper to catch human failure :) | 15:01 |
| elodilles | about the .gitreview patch needed manual change: IIRC we had a prompt discussion in the past and decided that it's not a big deal to update those 2 patches every six month | 15:04 |
| clarkb | elodilles: right I get that, the issue is occasionally (and usually when I'm touching stable) we have to modify the mechanics of the branch directly without a backport | 15:07 |
| clarkb | this is an example of that case. I think reviewers can see that an update to .gitreview to set the correct branch or update a git review option is fine without a backport | 15:07 |
| elodilles | clarkb: actually i also do stable fixes that are stable only and for those cases we (nova (and some other teams)) use 'stable-only' tags in commit message | 15:10 |
| clarkb | elodilles: yes but that isn't consistent and you never updated the bot to do that (which is really my complaint) | 15:11 |
| clarkb | you should disable the bot on your repos or update the bot | 15:12 |
| elodilles | clarkb: ok, i see your point | 15:12 |
| clarkb | personally I'd remove the lint rule entirely because the only way to discover the rule is to push code and have it fail first | 15:12 |
| elodilles | clarkb: I'll update the bot | 15:12 |
| elodilles | clarkb: well, in our team stable cores decided that this linter is useful for us (especially for reviewers). I understand your point, but i tend to look it from another perspective :) | 15:14 |
| clarkb | ok. In general I think commit message linters are a bad idea. Users don't know they will fail until after they have pushed (because typical workflow is make changes, run tox, commit, push) | 15:17 |
| sean-k-mooney | clarkb: we tried to strick a blance by having it non votign in check to give early feedback and block it only in gate | 15:19 |
| elodilles | clarkb: without the linter we did the very same: if a stable reviewer saw that something is not complete in the commit message (or a wrong patch was backported) then we asked the author to change the patch | 15:19 |
| sean-k-mooney | they can also run the lint check locally | 15:19 |
| gibi | clarkb: I think it is not impossible to add the current check as a tox target too (or part of pep8) so the extra push can be avioded | 15:19 |
| clarkb | elodilles: right but in the case of fixing the mechanics of a stable branch you wouldn't do that because it is fine as is | 15:19 |
| elodilles | clarkb: and that linter (script) can be also run locally | 15:19 |
| sean-k-mooney | most user do not backport | 15:19 |
| clarkb | sean-k-mooney: they can but literally no one expects they have to run the linter after committing | 15:19 |
| clarkb | this is why we removed the '.' at end of subject lines rule from hacking | 15:20 |
| sean-k-mooney | clarkb: well we have been trying to encurage people to use precommit more by the way | 15:20 |
| clarkb | oh wow. I would -2 that too :P | 15:20 |
| elodilles | gibi: we have it as an extra tox target now as stephenfin proposed it a couple of months ago | 15:20 |
| clarkb | (it doesn't handle dependencies properly and you'll completely bypass mirrors etc) | 15:20 |
| sean-k-mooney | clarkb: pre comiit is not a repleace fdor any of our ci | 15:21 |
| clarkb | sean-k-mooney: right but people run it locally and may have proxies or mirrors etc | 15:21 |
| sean-k-mooney | it just elimnates the excuse fo forgetting to run expected checks | 15:21 |
| clarkb | sean-k-mooney: the pre commit hook won't catch the commit issue errors though because it happens too soon right? | 15:21 |
| sean-k-mooney | in this case yes | 15:22 |
| sean-k-mooney | but it will run flake8 exctra | 15:22 |
| sean-k-mooney | it is too early for this backport check | 15:22 |
| sean-k-mooney | althogh technially | 15:22 |
| sean-k-mooney | you can configure it to run on push | 15:22 |
| sean-k-mooney | not on commit locally | 15:23 |
| clarkb | it would be better to run it after the commit like the gerrit chaneg id hook | 15:23 |
| clarkb | then people won't ask me why push is so slow to gerrit | 15:23 |
| sean-k-mooney | yes so if you configure it to run pre-commit on push instead of commit then you get that effect | 15:23 |
| sean-k-mooney | but its not the default | 15:23 |
| sean-k-mooney | and i dont think they have a post commit option | 15:24 |
| sean-k-mooney | clarkb: being able to run it on push instead of commit i think is relitively new to the tool | 15:24 |
| sean-k-mooney | https://pre-commit.com/#top_level-default_stages | 15:26 |
| sean-k-mooney | actully you might be able to add addtion stages https://pre-commit.com/#config-stages | 15:27 |
| sean-k-mooney | this is beyond my basic use of the tool to date | 15:27 |
| bauzas | sean-k-mooney: I'm not super happy either with pre-commit hooks | 15:28 |
| bauzas | sometimes I just wanna push | 15:28 |
| sean-k-mooney | bauzas: it removes a lot of the teedium with doing dev | 15:28 |
| bauzas | and I don't want it to be hold per the sake of something I don't care | 15:28 |
| sean-k-mooney | bauzas: you can bypass it | 15:28 |
| sean-k-mooney | you just set an env var on the push line | 15:28 |
| sean-k-mooney | or commit line | 15:29 |
| bauzas | sean-k-mooney: I told about *pre-commit* | 15:29 |
| bauzas | not pre-gerrit ;) | 15:29 |
| bauzas | and sorry the push was meaningless | 15:29 |
| bauzas | I meant commit | 15:29 |
| sean-k-mooney | yes you can overrid it if you just want to skip the checks | 15:29 |
| clarkb | what is the issue with running `tox` ? | 15:30 |
| sean-k-mooney | its slower in some cases and pre-commit is not just about running tests | 15:31 |
| sean-k-mooney | it can fix things like mixed tabs/spaces ectra | 15:31 |
| sean-k-mooney | ye are both going to "love" https://review.opendev.org/c/openstack/nova/+/806182 by the way | 15:32 |
| sean-k-mooney | based on your reaction to date :) | 15:32 |
| stephenfin | bauzas: git commit -n | 15:32 |
| bauzas | reminder : nova meeting in 28 mins-ish | 15:32 |
| bauzas | clarkb: yeah that's one of my concerns | 15:33 |
| bauzas | we push more and more checks into commit stage | 15:33 |
| bauzas | and we add more linters | 15:33 |
| sean-k-mooney | and we improve the quaity of the code and our developemt workflow as a result | 15:33 |
| stephenfin | don't make humans do things that machines are good at | 15:34 |
| clarkb | if tox and pre commit run the same checks they should run in about the same amount of time | 15:34 |
| stephenfin | yeah, pre-commit and 'tox -e fast8' will run in ~ the same time, but you need to remember to do the latter | 15:35 |
| sean-k-mooney | clarkb: in some cases yes | 15:35 |
| bauzas | stephenfin: I don't disagree, I'm just on clarkb's side on the fact that could be left to tox | 15:35 |
| clarkb | stephenfin: the problem is discoverablity. Humans don't know what all the checks are and some of them don't run until you get to zuul (commit message linting). Also some of it is severe nitpicking like having a '.' in the subject line or not | 15:35 |
| bauzas | and linter jobs could be non-voting | 15:35 |
| stephenfin | we have commit message linting? | 15:35 |
| bauzas | we do | 15:35 |
| clarkb | stephenfin: yes that is what started this entire conversation | 15:36 |
| bauzas | for backport checks | 15:36 |
| sean-k-mooney | bauzas: they could be but unless we are going to do that with pep8 im really not ok with that | 15:36 |
| stephenfin | outside of the cherry-picked from line? | 15:36 |
| clarkb | stephenfin: the bot that fixes your .gitreview file when you make a new branch got -1'd because its commit message wasn't good neough | 15:36 |
| sean-k-mooney | clarkb: yep | 15:36 |
| sean-k-mooney | the both should have had a stable only statement in that commit | 15:36 |
| clarkb | the bot predates the check :P | 15:36 |
| sean-k-mooney | i know but its not the first time we have modfied the bots patch | 15:37 |
| stephenfin | clarkb: which change is this, specifically? | 15:37 |
| sean-k-mooney | stephenfin: the one for the .gittreview file for the stable branch | 15:37 |
| stephenfin | for nova? | 15:37 |
| sean-k-mooney | yes | 15:37 |
| clarkb | https://review.opendev.org/c/openstack/nova/+/809759 | 15:37 |
| stephenfin | okay, that's what I was expecting to see. Where's the check for the '.' in the subject line? | 15:38 |
| stephenfin | Or is that another project? | 15:38 |
| clarkb | stephenfin: that was an old checker for hacking. I bring it up because after much fighting we finally conceded that any checks on commit messages are a bad idea because nothing checks them until it is too late | 15:39 |
| stephenfin | Ah, okay, thanks. I'm caught up now :) | 15:39 |
| clarkb | basically any linter checks for commit messages are doomed to fail | 15:39 |
| clarkb | because nothing runs the linters post commit | 15:39 |
| clarkb | (except for zuul) | 15:39 |
| sean-k-mooney | clarkb: by the way if i am runging tox its often after i commit | 15:40 |
| clarkb | ok I don't know anyone else that does that unless they are bisecting to find issues in already merged code | 15:40 |
| stephenfin | I agree that we should carve out an exception for these bot generated changes to stable | 15:40 |
| sean-k-mooney | or just fix them | 15:41 |
| clarkb | there is a semi related issue here with PBRs support for encoding semver requirements in commit messages | 15:41 |
| clarkb | that causes a ton of problems too because commit messages will be merged and pbr will disagree with the version someone tagged by hand and then you end up i na really weird spot | 15:41 |
| clarkb | (basically you need to be flexible with commit messages because you creat headaches when you try to enforce too many rules around them) | 15:42 |
| stephenfin | sean-k-mooney: they've always been that way though so it seems weird to let our custom script dictate what the commits for every other project looks like | 15:42 |
| * lyarwood jumps back on after the dentist | 15:42 | |
| sean-k-mooney | i semi agree but on the ohter hand hacking allows things i hate and blocks things i like yet we allow it to keep the porject semi consitent | 15:43 |
| lyarwood | I've likely missed something here but why does this matter if a core still needs to ACK the bot proposed patches? | 15:43 |
| lyarwood | and in ACK'ing these bot changes can add the [stable-only] tag? | 15:43 |
| sean-k-mooney | stephenfin: when we added the script we were hoping ti would gain adoption in other porjects too | 15:43 |
| lyarwood | it's not like these changes are merge automagically right? | 15:43 |
| lyarwood | merged* | 15:43 |
| sean-k-mooney | correct | 15:44 |
| sean-k-mooney | i dont see why the stable team cant fix the bot patches when they approve | 15:44 |
| clarkb | the whole point of having the bot is to reduce human overhead | 15:44 |
| stephenfin | clarkb: Agreed in general. This feels slightly different though. It was put in because dansmith was bothered by me proposing backports to multiple branches at once, in the fear that they'd merge in the wrong order or a patch higher up would be modified and the changes wouldn't be captured in later patches. Those still seems like sensible concerns that'd be easily missed by reviewers | 15:44 |
| clarkb | essentially we've got two bots that are in theory supposed to reduce overhead but in reality dobule it | 15:44 |
| lyarwood | well that's on the stable team who want the second bot surely? | 15:45 |
| lyarwood | I don't get the argument here tbh | 15:45 |
| lyarwood | we want the cherry-pick job and are happy with the overhead | 15:45 |
| lyarwood | surely that's enough? | 15:45 |
| clarkb | I've got two main concerns. The first is that after many yaers we still act like proposing a non backport change to stable branches is immediately wrong. This is frustraing for people like me who basically only do that when touching stable branches | 15:45 |
| clarkb | the second issue is that any linter applied to commit messages is problematic because you don't typically run linting post commit | 15:46 |
| sean-k-mooney | im more or less of the opipion that we keep the cherry pick lines and enforce them in a job or we done enforce them in a job and dont require them any more | 15:46 |
| sean-k-mooney | clarkb: well actully having the bot tell you its wrong to propose directly to stable was one of the goals of the script | 15:46 |
| sean-k-mooney | clarkb: its not that uncommon to have one off patches propsoed directly to stable | 15:47 |
| sean-k-mooney | i have -1'd patches a cople of time for that and explained that the issue need to be resolved in master first | 15:48 |
| elodilles | clarkb: just for the record, this cherry-pick-check is only in nova repository (well, and afaik cyborg adopted it as well), all the other repositories are not having it | 15:48 |
| lyarwood | clarkb: sorry back, I appreciate that the initial -1 is slightly off putting but again if the stable team looking after the repo are happy to do the work to fix this while they approve then I can't see an issue here. | 15:53 |
| opendevreview | Stephen Finucane proposed openstack/nova master: tools: Ignore bot-generated branch creation patches https://review.opendev.org/c/openstack/nova/+/810285 | 15:53 |
| stephenfin | lyarwood, sean-k-mooney, elodilles: eh? ^ | 15:54 |
| gibi | stephenfin: thanks! | 15:54 |
| stephenfin | no need to backport that, obv. It'll just avoid this discussion come stable/yoga creation | 15:54 |
| dansmith | stephenfin: can we not just agree on some flag for bot-generated things? | 15:54 |
| lyarwood | ha hackaroundFinucane has entered the chat | 15:54 |
| stephenfin | dansmith: Sure. I just don't want to fix the bot :) | 15:54 |
| dansmith | checking for those strings seems fragile, but I'll also say I agree with lyarwood and do not understand why humans can't just handle this | 15:55 |
| dansmith | are the bots always the same userid? | 15:56 |
| dansmith | i.e. checking for owner might just be easier | 15:56 |
| clarkb | I think there are a couple of bots but it is a small number | 15:56 |
| bauzas | reminder : 3 mins before the nova meeting | 15:57 |
| bauzas | honestly, I stopped discussing this | 15:58 |
| dansmith | I also think that assuming everything proposed against stable is wrong until proven right is the correct approach, just FYI | 15:58 |
| clarkb | ok I won't worry about the bot being told it is wrong then | 15:59 |
| opendevreview | Stephen Finucane proposed openstack/nova master: tools: Ignore bot-generated branch creation patches https://review.opendev.org/c/openstack/nova/+/810285 | 15:59 |
| stephenfin | dansmith: Good idea ^ | 15:59 |
| dansmith | stephenfin: ++ | 16:00 |
| bauzas | #startmeeting nova | 16:00 |
| opendevmeet | Meeting started Tue Sep 21 16:00:51 2021 UTC and is due to finish in 60 minutes. The chair is bauzas. Information about MeetBot at http://wiki.debian.org/MeetBot. | 16:00 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 16:00 |
| opendevmeet | The meeting name has been set to 'nova' | 16:00 |
| bauzas | #link https://wiki.openstack.org/wiki/Meetings/Nova#Agenda_for_next_meeting | 16:01 |
| bauzas | sorry for interupting you, folks :) | 16:01 |
| bauzas | who's around ? | 16:01 |
| dansmith | o/ | 16:01 |
| * stephenfin lurks | 16:01 | |
| lyarwood | \o | 16:01 |
| elodilles | o/ | 16:01 |
| gibi | o/ | 16:02 |
| bauzas | we have some large discussions today, let's start quickly | 16:02 |
| bauzas | #topic Bugs (stuck/critical) | 16:03 |
| bauzas | No Critical bug | 16:03 |
| bauzas | #link 13 new untriaged bugs (-0 since the last meeting): #link https://bugs.launchpad.net/nova/+bugs?search=Search&field.status=New | 16:03 |
| bauzas | no open bugs marked with xena-rc-potential tag #link https://bugs.launchpad.net/nova/+bugs?field.tag=xena-rc-potential | 16:03 |
| bauzas | please start marking release critical bugs with xena-rc-potential tag | 16:03 |
| bauzas | reminder, we are in RC phase | 16:03 |
| bauzas | meaning that we should focus on regression bugs | 16:03 |
| bauzas | yoga is now the master branch | 16:04 |
| bauzas | any bug to discuss ? | 16:04 |
| bauzas | #topic Gate status | 16:04 |
| bauzas | Nova gate bugs #link https://bugs.launchpad.net/nova/+bugs?field.tag=gate-failure | 16:04 |
| bauzas | we have a long list | 16:04 |
| bauzas | Placement periodic job status #link https://zuul.openstack.org/builds?project=openstack%2Fplacement&pipeline=periodic-weekly | 16:04 |
| bauzas | we now we failed because of a dependency issue | 16:05 |
| bauzas | know* we | 16:05 |
| bauzas | but now https://review.opendev.org/c/openstack/placement/+/810001 is merged | 16:05 |
| bauzas | so we can recheck | 16:05 |
| bauzas | any concern so far ? | 16:05 |
| bauzas | moving on | 16:06 |
| bauzas | Please look at the gate failures, file a bug, and add an elastic-recheck signature in the opendev/elastic-recheck repo (example: #link https://review.opendev.org/#/c/759967) | 16:06 |
| bauzas | #topic Release Planning | 16:06 |
| bauzas | (we'll discuss the placement dependency bump later) | 16:06 |
| bauzas | Release tracking etherpad #link https://etherpad.opendev.org/p/nova-xena-rc-potential | 16:06 |
| bauzas | as you can see, nothing fancy to tell | 16:06 |
| gibi | we need https://review.opendev.org/c/openstack/nova/+/810192 then an RC2 for nova | 16:06 |
| bauzas | most of our efforts should go to merging bugfixes | 16:07 |
| bauzas | gibi: right, I forgot about it | 16:07 |
| bauzas | strangely, LP says Fix Released for Xena https://bugs.launchpad.net/nova/+bug/1944111 | 16:08 |
| gibi | for some reason the bug is not showing up in the rc critical query | 16:08 |
| gibi | feels like an LP bug around branching | 16:08 |
| bauzas | yup | 16:08 |
| bauzas | will add it in the etherpad for tracking | 16:09 |
| gibi | already done :) | 16:09 |
| bauzas | hah | 16:09 |
| bauzas | naïce ;) | 16:09 |
| bauzas | ok, let's wait a bit for releasing RC2 | 16:09 |
| bauzas | We now have RC1 releases for both Nova and Placement #link https://review.opendev.org/c/openstack/releases/+/808706 and https://review.opendev.org/c/openstack/releases/+/808713 | 16:10 |
| bauzas | We will need a Placement RC2 release due to #link https://review.opendev.org/c/openstack/placement/+/810001 | 16:10 |
| bauzas | https://review.opendev.org/c/openstack/placement/+/810193 needs special treatme | 16:10 |
| bauzas | treatment | 16:10 |
| bauzas | +2d now | 16:10 |
| gibi | we have the last RC deadline at 1st of Oct afaik | 16:11 |
| bauzas | yeah | 16:11 |
| gibi | so yes, we can hold up RC2 a bit to see if anything else pops up | 16:11 |
| bauzas | hence me saying let's hold a bit for a RC2 proposal | 16:11 |
| bauzas | ya | 16:11 |
| bauzas | Remember to propose regression bugfixes for a new RC with nova-xena-rc-potential | 16:11 |
| bauzas | #topic Review priorities | 16:12 |
| bauzas | https://review.opendev.org/q/status:open+(project:openstack/nova+OR+project:openstack/placement)+label:Review-Priority%252B1 | 16:12 |
| bauzas | this is absolutely empty | 16:12 |
| bauzas | cores, please enjoy this new button until we discuss other ways to use it at the PTG | 16:12 |
| bauzas | is there anyone wanting to have their changes be a priority for reviews ? | 16:13 |
| bauzas | (that's your time, folks) | 16:13 |
| artom | Wait, only cores can set the priority label? | 16:13 |
| bauzas | with the current implementation, yes | 16:14 |
| bauzas | I'm about to propose other possibilities at the PTG | 16:14 |
| bauzas | (which reminds me I have to write something in the etherpad) | 16:14 |
| artom | Can a non-core propose the label, or is the whole thing owned entirely by cores? | 16:14 |
| bauzas | artom: dude, that's the whole discussion we had in the related doc change :) | 16:15 |
| gibi | artom: this is the currently agreed process https://review.opendev.org/c/openstack/nova/+/792357 but sure we can discuss it on the PTG | 16:15 |
| dansmith | surely it will be totally useless if not curated by a smaller group right? | 16:15 |
| artom | Sorry, not opening pandora's box again, just catching up | 16:15 |
| bauzas | I'm happy to see people offering alternatives :) | 16:15 |
| dansmith | in bug trackers where everyone can set priority, people set their bugs to be "urgent" all the time | 16:15 |
| opendevreview | Takashi Natsume proposed openstack/nova master: Update min supported service version for Yoga https://review.opendev.org/c/openstack/nova/+/809932 | 16:15 |
| dansmith | I can't imagine it will be useful otherwise | 16:15 |
| opendevreview | Takashi Natsume proposed openstack/nova master: Update min supported service version for Yoga https://review.opendev.org/c/openstack/nova/+/809932 | 16:15 |
| bauzas | dansmith: I had an alternative proposal I left in the comments but we can discuss this at the PTG (and I'll propose a documentation change) | 16:16 |
| dansmith | ack | 16:16 |
| artom | Essentially, for my own selfish needs, all I can do with the Priority label is query it, and look at those reviews in case my opinion on them is of any use to anyone, right? | 16:16 |
| bauzas | artom: for the moment, the idea is that cores make some review vows when setting the label | 16:16 |
| gibi | artom: if you help merging reviews in priority then cores will have more bandwidth to look at other reviews including yours :D | 16:17 |
| bauzas | meaning they engage themselves to review this patch in question | 16:17 |
| bauzas | but I'm not sure we should continue this conversation now, we have some agenda left | 16:17 |
| bauzas | (and a stretched time) | 16:17 |
| artom | Understood. | 16:18 |
| bauzas | moving on | 16:18 |
| bauzas | #topic PTG Planning | 16:18 |
| bauzas | every info is in the PTG etherpad #link https://etherpad.opendev.org/p/nova-yoga-ptg | 16:19 |
| bauzas | If you see a need for a specific cross project section then please let me know (gibi or bauzas) | 16:19 |
| bauzas | gibi: I jinxed you :p | 16:19 |
| bauzas | at least we can see regular updates from sean-k-mooney :) | 16:19 |
| gibi | I expect more and more topics as we close to the PTG date | 16:20 |
| gibi | I think the current list is normal given we have a month still | 16:20 |
| bauzas | absolutely right | 16:20 |
| gibi | (a bit less of a month | 16:20 |
| gibi | ) | 16:20 |
| bauzas | I'm not afraid of having extra time | 16:20 |
| gibi | me neither | 16:20 |
| bauzas | but I'd say, take this month as an opportunity to start designing your features, so you can raise your questions at the PTG | 16:21 |
| bauzas | anyway, moving on | 16:21 |
| bauzas | #topic Stable Branches | 16:21 |
| bauzas | (that could last a bit) | 16:21 |
| bauzas | nova's stable/ussuri and stable/train are blocked (due to latest virtualenv uses latest setuptools which removed use_2to3) | 16:22 |
| bauzas | the future proof solution would be to pin virtualenv during tox install for stable branches, otherwise new errors can appear with every new release of setuptools, virtualenv, etc | 16:22 |
| bauzas | until we decide about the right solution we can maybe set lower-constraints job as non-voting ( https://review.opendev.org/809955 ) | 16:22 |
| bauzas | probably placement branches have the same errors | 16:22 |
| bauzas | elodilles: floor is yours | 16:22 |
| gibi | bauzas: yes, this is the same error | 16:22 |
| opendevreview | Merged openstack/nova stable/xena: Add missing __init__.py in nova/db/api https://review.opendev.org/c/openstack/nova/+/810192 | 16:22 |
| gibi | it affects a lot of projects that still uses decorator 3.4 as dep | 16:22 |
| gibi | nova affected from ussuri backwards | 16:22 |
| gibi | placement affected all the way to master | 16:23 |
| gibi | (master fix landed) | 16:23 |
| lyarwood | ah I missed this review sorry | 16:23 |
| * lyarwood opens | 16:23 | |
| bauzas | super awesome | 16:23 |
| sean-k-mooney | we may want to consider droping that dep at some point | 16:23 |
| bauzas | we actually have a longer explanation in the open discussion section | 16:23 |
| bauzas | let's just hold this discussion until that point | 16:23 |
| sean-k-mooney | although we are unlikely to hit the same issue again | 16:24 |
| bauzas | (which we will have 30 mins for) | 16:24 |
| bauzas | #topic Sub/related team Highlights | 16:24 |
| bauzas | Libvirt (bauzas) | 16:24 |
| lyarwood | bauzas: happy to take over the libvirt part now you're PTL btw | 16:24 |
| bauzas | lyarwood: awesome | 16:24 |
| bauzas | I was asking for it | 16:24 |
| lyarwood | bauzas: not that I have anything for today but we have a few things this cycle | 16:25 |
| bauzas | any stuff to raise ? | 16:25 |
| bauzas | kk | 16:25 |
| bauzas | #info lyarwood to chair the libvirt subteam by now | 16:25 |
| bauzas | lyarwood: thanks for offering your name | 16:25 |
| *** rpittau is now known as rpittau|afk | 16:25 | |
| artom | Don't take it in vain now | 16:25 |
| bauzas | #topic Open discussion | 16:26 |
| bauzas | one last paperwork bit | 16:26 |
| bauzas | (gibi): release liaison role | 16:26 |
| gibi | so | 16:26 |
| gibi | bauzas: is now the PTL and the release liaison as well | 16:26 |
| gibi | which is suboptimal | 16:26 |
| bauzas | which means I'm also a bottleneck now | 16:26 |
| gibi | do we have a volunteer to take that role over? | 16:26 |
| gibi | it is not hard, you will get cc-d to release proposal patches to check and approve | 16:27 |
| bauzas | tbc, the release liaison role is about reviewing patches from the release team | 16:27 |
| bauzas | until either the PTL or the release liaison approves the patch, it can't land | 16:27 |
| sean-k-mooney | i mean i can do it if no one else wants too i really dont mind | 16:28 |
| bauzas | example https://review.opendev.org/c/openstack/releases/+/808706 | 16:28 |
| sean-k-mooney | i keep an eye on the os-vif ones anyway but someone on the stable team might make more sense | 16:28 |
| bauzas | sean-k-mooney: appreciated | 16:28 |
| bauzas | I'll make the changes in the appropriate repo so you'd get automatically CC'd | 16:29 |
| gibi | sean-k-mooney: thank you | 16:29 |
| sean-k-mooney | no worries | 16:29 |
| * bauzas just has to figure out which specific file to update in which specific repo :D | 16:29 | |
| elodilles | well, as I also a release core and also propose nova releases lately it would be weird to propose and approve my own patches and then +W o:) | 16:29 |
| bauzas | elodilles: heh, depending whether you're schizophrenic, this could work | 16:30 |
| elodilles | :D | 16:30 |
| bauzas | ok, moving on | 16:31 |
| bauzas | sean-k-mooney: thanks again | 16:31 |
| bauzas | now the big discussion | 16:31 |
| bauzas | pasting the whole section | 16:31 |
| bauzas | (gibi): gathering opinions about the current lower-constraints failure. | 16:31 |
| bauzas | bottom line: on stable branches we are installing tox which installs virtualenv which bundles setuptools unconstrained. This now leads to that we cannot install decorator 3.4.0 on stable any more as it depends on "user_2to3" from setuptools but the recent setuptools 58.0 removed support for that. | 16:31 |
| bauzas | Options to resolve the situation bump decorator major version from 3.4.0 to 4.0.0 on stable branches. Does it against stable policy? pin virtualenv version on stable during tox install disable lower-constraints testing | 16:32 |
| bauzas | shit, I pasted wrong | 16:32 |
| bauzas | gibi: your turn | 16:32 |
| bauzas | explain the 3 options | 16:32 |
| gibi | let me untangle that | 16:32 |
| gibi | so option 1) bump major version of decorator from 3.4 to 4.0 on stable. Is it allowed on stable? | 16:32 |
| gibi | option 2) pin virtualenv during tox install | 16:33 |
| gibi | option 3) disable lower-constraints job | 16:33 |
| gibi | I personally think that using unconstrained setuptools on stable is dangerous | 16:33 |
| gibi | so I would go with 2) long turn | 16:33 |
| gibi | term | 16:33 |
| artom | Yeah, seems like 2 is the safest... what's the danger with it? Is there a catch? | 16:33 |
| lyarwood | Agreed, 2 would be my choice | 16:33 |
| bauzas | gibi: let's explain which stable branches again are impacted | 16:33 |
| bauzas | you already told but this doesn't harm to tell again | 16:34 |
| gibi | so in nova we are impacted stable/ussuri and older | 16:34 |
| sean-k-mooney | gibi: technially i dont think we are allowe to bump miniums on stable on the other hand we already did it a while ago for the inital lower constraitnts fix | 16:34 |
| gibi | on placement we are impacted on master but the fix has been landed | 16:34 |
| gibi | on stable/xena we could release RC2 with the bump | 16:34 |
| elodilles | I also like option 2, and actually found some (abandoned) trial from the past that was similar https://review.opendev.org/q/topic:constrain-tox-install | 16:34 |
| gibi | but on stable/wallaby and back the same quertion applies | 16:34 |
| sean-k-mooney | from a disto point of view 2 is not great | 16:35 |
| sean-k-mooney | since they cannot pin them the same way | 16:35 |
| bauzas | option 1 has a question I can't answer | 16:35 |
| sean-k-mooney | but most distros also dont use lower constratints | 16:35 |
| gibi | does LTS distros use unconstrained setuptools as well? | 16:35 |
| bauzas | can we deliver a .y release by bumping the decorator dependency without breaking the semver rules ? | 16:35 |
| sean-k-mooney | rhel ignored the upper and lower constraitnts | 16:36 |
| sean-k-mooney | debian used to look at lower | 16:36 |
| sean-k-mooney | ubunut used to look at upper | 16:36 |
| sean-k-mooney | we can try 2 but i feel like we are going to end up with 3 | 16:36 |
| gibi | sean-k-mooney: sure we could end up with 3 for different reasons :) | 16:37 |
| artom | So 2 for distros means "maintain this old version of virtualenv in your repos", right? | 16:37 |
| elodilles | bauzas: I think we can, but it's probably not fortunate. semver says that req changes needs MINOR / .y version bump | 16:37 |
| artom | But... don't we already have upper-constraints that does the exact same thing? | 16:37 |
| gibi | artom: yes, as virtualenv bundles setuptools | 16:37 |
| bauzas | elodilles: so option 1 doesn't break stable policy per se | 16:37 |
| gibi | artom: this is tricky as the tox install in our CI does not use constraint file | 16:38 |
| gibi | artom: we install tox, then with tox we install deps | 16:38 |
| gibi | artom: but when we install tox it pulls in setuptools | 16:38 |
| artom | Oh, so this is before any -constraints is applied | 16:38 |
| bauzas | the problem is with the venv, not the dependencies | 16:39 |
| bauzas | artom: right | 16:39 |
| bauzas | because of the bundle | 16:39 |
| elodilles | bauzas: I also haven't found that written, but I remember that we tried to avoid req changes | 16:39 |
| bauzas | but, there are ways to create venvs without bundling setuptools, right? | 16:39 |
| elodilles | bauzas: and the problem is that we will face the same issue whenever something new things comes in | 16:40 |
| artom | Wait, I though the decorator==3.4.0 install happened *inside* the venv, as a dependency? | 16:40 |
| bauzas | elodilles: agreed, option 1 only fixes the decorator issue and doesn't resolve any other issue | 16:40 |
| bauzas | I'm also in favor of option 2 | 16:40 |
| gibi | artom: to have a venv where you can install deps, you have to have the venv package installed | 16:41 |
| bauzas | but I wonder whether we could just tell tox to create venvs without bundling setuptools | 16:41 |
| stephenfin | just weighing in, 3 isn't an ideal solution either | 16:41 |
| gibi | bauzas: I think if you have a venv without setuptools then you dont have pip in the venv either | 16:41 |
| gibi | bauzas: so you cannot install additional things with pip | 16:41 |
| bauzas | gibi: are you sure ? | 16:41 |
| gibi | bauzas: 75% | 16:42 |
| bauzas | I think it does install pip | 16:42 |
| gibi | I think pip depends on setuptools | 16:42 |
| stephenfin | in this case it's the lower-constraint that has broken, however, it's conceivable that the upper constraint for a particularly old release could also become incompatible with setuptools | 16:42 |
| gibi | but I could be wrong | 16:42 |
| gibi | stephenfin: good point | 16:42 |
| artom | gibi, right, so we install tox on the "host", which pulls in unconstrained setuptools, which then break installing decorator==3.4.0, my question is, aren't we installing decorator==3.4.0 inside the venv? | 16:42 |
| gibi | artom: hm, | 16:42 |
| artom | So it should be independent of what's on the "host"? | 16:42 |
| gibi | artom: you have a point | 16:42 |
| gibi | artom: for some reason our ci install tox already in a venv but I lost following that track | 16:43 |
| stephenfin | artom: it installs setuptools in the venv also, right? | 16:43 |
| artom | Don't know... but then it'd be a separate venv for the actual nova install that pulls in decorator... | 16:44 |
| gibi | I agree that something is missing from our understanding about venvs and tox install | 16:44 |
| stephenfin | >>> setuptools.__file__ | 16:45 |
| stephenfin | '/home/stephenfin/Development/openstack/nova/.tox/py36/lib/python3.6/site-packages/setuptools/__init__.py' | 16:45 |
| sean-k-mooney | https://tox.readthedocs.io/en/latest/config.html#conf-requires | 16:45 |
| sean-k-mooney | if we need to pin it we can pin the setuptools and pip version with requires | 16:45 |
| bauzas | I think it's doable | 16:45 |
| clarkb | tox pulls in virtualenv, virtualenv bundles setuptools for the new virtualenvs it makes | 16:46 |
| gibi | sean-k-mooney: good finding | 16:46 |
| sean-k-mooney | it is but fungi and clarkb advised against doing that | 16:46 |
| elodilles | in my abandoned trial in the past i tried to use upper-constraints.txt for tox install, it probably could solve the issue if that works (i don't remember whether it worked) | 16:46 |
| clarkb | virtualenv has an escape hatch for that but I'm not sure how tox exposes that (if at all) | 16:46 |
| bauzas | https://paste.opendev.org/show/809477/ | 16:46 |
| clarkb | you need to pin virtualenv is the tldr if you are going to pin something | 16:46 |
| clarkb | but you have to do it when installing tox | 16:46 |
| bauzas | you can create a venv without setuptools and pip it later | 16:46 |
| clarkb | right but does tox support any of that? | 16:46 |
| clarkb | as it is tox making the virtualenv automatically | 16:47 |
| bauzas | tox uses the venv module, right?N | 16:47 |
| clarkb | not by default I think it can | 16:47 |
| artom | clarkb, gibi, ah, that's the missing understanding, when virtualenv creates a venv, it automatically stuffs the setuptools version it came with in that venv... right? | 16:47 |
| clarkb | basically oyu have to see how much of this is configurable on the tox side to do what you want | 16:47 |
| clarkb | artom: yes exactly | 16:47 |
| opendevreview | Stephen Finucane proposed openstack/nova master: db: Add migration to resolve shadow table discrepancies https://review.opendev.org/c/openstack/nova/+/805738 | 16:47 |
| opendevreview | Stephen Finucane proposed openstack/nova master: tests: Walk database migrations in correct order https://review.opendev.org/c/openstack/nova/+/810291 | 16:47 |
| bauzas | clarkb: you're right, I need to dig into tox | 16:48 |
| stephenfin | This looks like good background reading https://tox.readthedocs.io/en/latest/example/package.html | 16:48 |
| gibi | clarkb: thanks | 16:48 |
| artom | Sounds like we have no choice but to pin virtualenv then | 16:48 |
| stephenfin | Sounds like the escape hatches we need might be there | 16:48 |
| artom | Unless there's a way to tell virtualenv which setuptools version to install in the venvs in creates | 16:48 |
| fungi | if you want tox to use venv as the virtualenv backend, these days the trick is to export VIRTUALENV_CREATOR = venv in the inherited setenv in your tox.ini | 16:48 |
| fungi | there used to be a tox-venv plugin, but that was deprecated/abandoned once virtualenv grew the option to call out to the venv module itself | 16:49 |
| bauzas | can we use https://tox.readthedocs.io/en/latest/config.html#conf-requires ? | 16:49 |
| fungi | so it's not tox calling venv directly, but rather tox calling virtualenv and then virtualenv being told to use the venv module to create the env rather than its internal implementation | 16:49 |
| sean-k-mooney | i feel like the complexity of maintianing this pining outways the usefullness of the job | 16:50 |
| bauzas | sean-k-mooney: that's the problem with option 2 | 16:50 |
| bauzas | ideally, I'd like us to maitain setuptools in our reqs | 16:50 |
| bauzas | maintain* even | 16:50 |
| bauzas | but this doesn't sound easy | 16:50 |
| artom | So the value of the job is what - guarantee that will still work with the oldest feasible versions of things? | 16:51 |
| artom | (lower-constraints I mean) | 16:51 |
| stephenfin | sean-k-mooney: We will always have pinning in the form of upper-constraints though | 16:51 |
| sean-k-mooney | yes basically to make sure when we backport we dont raise the requiremetns as a result | 16:51 |
| clarkb | the correct way to maintain setuptools in your requirements is via the pyproject file | 16:51 |
| sean-k-mooney | stephenfin: right but for stable we are not allowed to raise miniums | 16:51 |
| clarkb | not requirements | 16:51 |
| stephenfin | and? we can't raise maximums either | 16:52 |
| clarkb | the new pyproject file allows you to specify build time deps (possibly even tools other than setuptools) | 16:52 |
| clarkb | I don't know what would be involved to make all of that work with pbr though | 16:52 |
| bauzas | clarkb: how other projects consider this issue ? do they want to pin setuptools as well ? | 16:53 |
| bauzas | or do they just fix the decorator issue ? | 16:54 |
| bauzas | (and backport without specific care) | 16:54 |
| bauzas | I guess we're not alone in the dark | 16:54 |
| clarkb | msot of the fixes I've seen have been to fix the dependencies | 16:54 |
| bauzas | and about backports ? | 16:54 |
| clarkb | openstack governance had to switch pydot2 to pydot for example | 16:54 |
| clarkb | zuul-jobs dropped support for old python3.5 which allowed it to use a newer version of a lib | 16:55 |
| clarkb | I don't know how backports or stable issues have been handled | 16:55 |
| bauzas | gibi: I guess you have to write something and bubble it to the community for cross-project thinking | 16:56 |
| gibi | bauzas: I can write up a summary from today on the ML | 16:57 |
| bauzas | that could become an epic saga, but it's worth starting it | 16:57 |
| bauzas | gibi: appreciated, here lemme summarize for the audience | 16:57 |
| bauzas | option 2 seems to be the better, but we struggle finding a good way to write it | 16:57 |
| artom | Stupid question, but could we not just install virtualenv==<version we want> first, and *then* tox? | 16:58 |
| bauzas | option 1 seems to be the alternative, option 3 seems not accepted | 16:58 |
| artom | Or would tox just upgrade? | 16:58 |
| gibi | artom: that could be also something to try | 16:58 |
| bauzas | artom: virtualenv pulls the latest setuptools IIRC | 16:58 |
| bauzas | but that's a flag | 16:59 |
| gibi | bauzas: old virtualenv will pull old setuptools I guess | 16:59 |
| bauzas | again, the problem is how to trigger this flag thru tox | 16:59 |
| bauzas | gibi: I remember having played with it before and you're right | 16:59 |
| bauzas | but you can explicitely specific to pull the latest setuptools | 17:00 |
| bauzas | either way, we're at time | 17:00 |
| bauzas | gibi: thanks for writing the summary and raising it to the community | 17:00 |
| bauzas | folks, wrapping up | 17:00 |
| bauzas | thanks | 17:00 |
| bauzas | #endmeeting | 17:00 |
| opendevmeet | Meeting ended Tue Sep 21 17:00:40 2021 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 17:00 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/nova/2021/nova.2021-09-21-16.00.html | 17:00 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/nova/2021/nova.2021-09-21-16.00.txt | 17:00 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/nova/2021/nova.2021-09-21-16.00.log.html | 17:00 |
| gibi | bauzas: thanks | 17:00 |
| elodilles | thanks bauzas o/ | 17:01 |
| stephenfin | I've already tagged people, but there are some necessary fixes for the DB migration tests here that we should probably merge before I forget about them https://review.opendev.org/c/openstack/nova/+/810291 | 17:02 |
| stephenfin | The alternative is to rediscover this stuff once someone adds a DB migration in Yoga or later | 17:02 |
| stephenfin | (https://review.opendev.org/c/openstack/nova/+/805738 proves that they work) | 17:02 |
| bauzas | stephenfin: I just made use of the review-priority label for your change :) | 17:03 |
| gibi | stephenfin: thanks, I will try to get to them | 17:03 |
| opendevreview | Stephen Finucane proposed openstack/nova master: tools: Ignore bot-generated branch creation patches https://review.opendev.org/c/openstack/nova/+/810285 | 17:04 |
| stephenfin | lyarwood: sorry for the dumb typo in that fixed. Bash is the worst. Fixed ^ | 17:05 |
| opendevreview | Balazs Gibizer proposed openstack/placement stable/xena: [WIP] try to pin setuptools via tox.ini https://review.opendev.org/c/openstack/placement/+/810293 | 17:05 |
| gibi | trying [tox]/requires ^^ | 17:07 |
| * gibi ends his day | 17:12 | |
| bauzas | gibi: me too | 17:12 |
| bauzas | \o | 17:12 |
| gibi | it seems [tox]requires does not help us pinning setuptools | 17:53 |
| fungi | just to clarify, very old virtualenv pulls old setuptools, but a while back virtualenv switched to pulling whatever the most recent setuptools available is unless you can tell it not to, so just pinning to a slightly older virtualenv doesn't help as it will still insist on grabbing the latest available setuptools | 18:46 |
| fungi | also there's the fact that this isn't purely a ci side problem, you can't really know or control what version of setuptools a user will have | 18:47 |
| fungi | because it's considered part of the build environment not part of the runtime environment, so python packaging doesn't handle it the same way with the same expectations | 18:48 |
| fungi | the idea historically was that there were mechanisms to insist on a minimum acceptable version of build dependendencies and raise errors if they weren't new enough, but not really any reliable way to indicate maximum acceptable versions | 18:51 |
| fungi | of course, that was also predicated on build dependencies maintaining backward compatibility, and setuptools removing a feature is what's put us in this situation | 18:52 |
| clarkb | upstream pypa seems to say everyone should use pyproject.toml to address this | 19:00 |
| clarkb | which is why it is safe for them to drop functionality like that | 19:00 |
| *** hemna7 is now known as hemna | 20:24 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!