Monday, 2023-10-02

*** efried1 is now known as efried05:18
bauzasgood morning Nova06:58
gibio/07:26
dvo-plvHello, I would like to remind about to take a look on the reproposed spec file ? https://review.opendev.org/c/openstack/nova-specs/+/89592407:45
dvo-plvand check dependent patch for it https://review.opendev.org/c/openstack/nova/+/87607507:45
bauzasyup, specs reviews is on my agenda this week07:53
*** Continuity__ is now known as Continuity08:03
opendevreviewsean mooney proposed openstack/nova master: remove py2 only syntax  https://review.opendev.org/c/openstack/nova/+/89698409:08
opendevreviewsean mooney proposed openstack/nova master: remove use of % format strings  https://review.opendev.org/c/openstack/nova/+/89698509:08
opendevreviewsean mooney proposed openstack/nova master: pyupgrade py 3.0 -> 3.6  https://review.opendev.org/c/openstack/nova/+/89698609:08
opendevreviewsean mooney proposed openstack/nova master: add precommit pyupgrade hook and tox env  https://review.opendev.org/c/openstack/nova/+/89698709:08
opendevreviewsean mooney proposed openstack/nova master: ignore pyupgrade series in git blame  https://review.opendev.org/c/openstack/nova/+/89698809:08
opendevreviewsean mooney proposed openstack/nova master: ignore pyupgrade series in git blame  https://review.opendev.org/c/openstack/nova/+/89698809:09
opendevreviewsean mooney proposed openstack/nova master: ignore pyupgrade series in git blame  https://review.opendev.org/c/openstack/nova/+/89698809:10
dvo-plvbauzas, good to hear,thank you09:26
opendevreviewStephen Finucane proposed openstack/nova master: doc: Remove crud from conf.py file  https://review.opendev.org/c/openstack/nova/+/89622410:20
opendevreviewStephen Finucane proposed openstack/nova master: pre-commit: Update plugin versions  https://review.opendev.org/c/openstack/nova/+/89622610:20
opendevreviewStephen Finucane proposed openstack/nova master: tox: Use pre-commit for pep8 target  https://review.opendev.org/c/openstack/nova/+/89622710:20
opendevreviewStephen Finucane proposed openstack/nova master: pre-commit: Add mypy  https://review.opendev.org/c/openstack/nova/+/89622810:20
opendevreviewStephen Finucane proposed openstack/nova master: Blacken codebase  https://review.opendev.org/c/openstack/nova/+/89622910:20
opendevreviewStephen Finucane proposed openstack/nova master: pre-commit: Integrate black  https://review.opendev.org/c/openstack/nova/+/89623010:20
opendevreviewStephen Finucane proposed openstack/nova master: Ignore black change  https://review.opendev.org/c/openstack/nova/+/89623110:20
opendevreviewStephen Finucane proposed openstack/nova master: pre-commit: Use native hacking pre-commit hook  https://review.opendev.org/c/openstack/nova/+/89700510:20
*** Continuity__ is now known as Continuity10:24
opendevreviewsean mooney proposed openstack/nova master: remove deprecated vmware virt driver  https://review.opendev.org/c/openstack/nova/+/89701712:38
opendevreviewMerged openstack/os-vif stable/2023.2: Update TOX_CONSTRAINTS_FILE for stable/2023.2  https://review.opendev.org/c/openstack/os-vif/+/89406212:55
*** haleyb_out is now known as haleyb13:33
stephenfinbauzas: I'm not sure I understand your comment on https://review.opendev.org/c/openstack/nova/+/896226/? Does my reply make sense?14:56
bauzasstephenfin: tbc, I'm not a big fan of pre-commit hooks14:57
bauzasthey slow our development process and avoid new contributors to easily push some code asking for help14:57
stephenfinbut you don't need to use them14:58
bauzasin general, I do atomic commits and then I squash14:58
bauzasthe more pre-commit hooks we add, the slower a git commit happens14:58
stephenfinonly if you enable them14:58
stephenfinwhich I'm guessing you don't?14:58
bauzassure, but you're adding them by default14:58
bauzaswhich relates to my other concern14:59
stephenfinI am?14:59
bauzasand yeah, I know I can disable hooks14:59
stephenfinhooks aren't enabled by default. It's a manual step to install them14:59
bauzasthat's why I'm proposing an half-way solution, which is to use pre-push hooks14:59
bauzasit's an automatic step on my env15:00
stephenfinbefore we talk about that, back up: where is this being added by default?15:00
stephenfinif you don't run 'pre-commit install' the hooks are irrelevant15:00
bauzasI need then to check something15:03
bauzasbecause I'm sure it's pulling pre-commit without my request15:03
stephenfinI'm very confused. Do you have pre-commit installed? If not, it isn't capable of installing itself or anything like that15:04
bauzasI do15:05
stephenfinOkay, if it's annoying couldn't you just remove it? The CI will still catch things for you15:05
bauzasand yeah I wonder how that package went into my env15:06
bauzasstephenfin: I surely can remove that package15:06
bauzasstephenfin: can we actually discuss that topic into our PTG sessions if you can attend ? black is a different matter, but I'd like us to agree on what are the expectations of a git commit15:07
bauzasI'm not sure everyone talks the same language15:07
stephenfinWe already have expectations of a git commit. The hard ones are encoded in the pep8 job. Nothing has changed15:11
stephenfinI can understand the merit of discussing the black stuff there, but pre-commit is simply a wrapper around tooling that we've already agreed on (autopep8, hacking, mypy). Do we need 20+ people discussing a tool like that?15:12
clarkbdid precommit fix the problem of not installing tools from pypi?15:14
stephenfinThat'd be a non-goal since PyPI is a Python-specific repository and pre-commit is intentionally language agnostic15:15
stephenfinat least, I imagine that to be the case? tbh I've never looked :)15:15
bauzasI just think that not a lot of people exactly understand which tools are run and why15:18
bauzasand if they see a pre-commit YAML file, they'll pull it15:18
bauzasbut if we are clear on the line that pre-commit is optional, then I question the need15:19
sean-k-mooneybauzas: they way we use pre-commit now its entirly optional to install the hook locally and all automations are enforced in the tox jobs already15:19
bauzasfor our CI, I guess ?15:19
bauzassean-k-mooney: the way it's done is that *if* you have pre-commit installed, then you're tied to its rules15:20
sean-k-mooneyyes but the pep8 job aslo runs the same checks15:21
sean-k-mooneyas does tox -e pep815:21
bauzassean-k-mooney: sure, but then I question the benefits15:22
sean-k-mooneyits faster for one15:22
sean-k-mooneyvs what we are doing today15:22
sean-k-mooneyyou can also skip pre-commit whenever you want with --no-verify15:23
sean-k-mooneyif you have it installed 15:23
clarkbhow is it faster if it runs the same checks?15:23
sean-k-mooneyif you dont have it installed the you can run it with tox15:23
sean-k-mooneyand not install it or change your workflow15:23
sean-k-mooneyclarkb: by default its flake8 check just the files you modifed15:23
sean-k-mooneyso its the same as our old fast8 tox env15:24
sean-k-mooneyfor other things its not neessisarly any faster15:24
dansmithit's still faaar too slow for run-on-commit for me :)15:24
bauzasI just feel we absolutely need to amend https://docs.openstack.org/nova/latest/contributor/development-environment.html#using-the-pre-commit-hook15:24
bauzasto make it optional15:24
sean-k-mooneyit is optional15:25
bauzasand specify how to skip it15:25
bauzassean-k-mooney: our docs aren't that clear15:25
sean-k-mooney if you dont install it then it does not run15:25
sean-k-mooneyif you do its just --no-verify15:25
sean-k-mooneybut we can updated it to be more clear15:25
melwittyou can also just do -n15:25
bauzasI think I can and shall update that docv15:26
melwittand the doc was clear enough to me, fwiw. i.e. I understood pre-commit doesn't happen unless you install the pre-commit package15:26
sean-k-mooneymelwitt: oh good to know15:26
sean-k-mooneyyep "This must be enabled locally to function:"15:26
sean-k-mooneywas ment to capture that in the doc15:26
bauzasthen I'll remove my vote on the pre-commit additions change15:27
melwitt(I'm not objecting to improving the doc btw, just saying at least to me I've used the doc and it made sense for me)15:27
sean-k-mooneybauzas: for what its worht i have other related but seperate changes 15:28
sean-k-mooneyhttps://review.opendev.org/c/openstack/nova/+/89698715:28
sean-k-mooneyi was going to add a "tech-debt reduction and code modernization" topic to the ptg (items for this seession would by hyperv and vmware driver removal. black and pyupgrade)15:32
sean-k-mooneywe have blueprints filed for at least 3 of those. https://blueprints.launchpad.net/nova/+spec/remove-hyperv-driver https://blueprints.launchpad.net/nova/+spec/pyupgrade and https://blueprints.launchpad.net/nova/+spec/remove-vmware-support15:34
sean-k-mooneyi dont know if we have or need one for automated formating but we can at least have the conversation at the ptg15:34
bauzassean-k-mooney: I'd be in favor of more fine-scoped topic names15:35
stephenfinbauzas: Thanks. Good to hear that confusion is cleared up (y)15:35
dansmithI'm prepared to die on black hill15:35
bauzasat least for clarifying the intent and clearly timekeep on subjects15:35
bauzasone topic I could add could be "volume attachment improvements" but that's exactly kind of the same vague bucket :)15:36
sean-k-mooneyi wanted to group related topics15:36
sean-k-mooneyhence the sub items15:36
bauzassean-k-mooney: if you don't mind, stick with items per each, and I'll just make sure we loop thru them as a group15:37
stephenfindansmith: You'll be happy to hear sean-k-mooney also does not like the spreading of arguments across multiple lines 0:) Apparently there's some way to change that but I didn't look into it yet15:37
sean-k-mooneythose 3 are all tech debth reduction and as an fyi i have fully implemted pyupgrade adn vmware removal 15:37
bauzas(fwiw, the vmware one seems to me quite non-controversial :) )15:38
sean-k-mooneyi have one open question on it15:38
sean-k-mooneybaiscally i intentionl didn not make any api changes but i proably should15:38
bauzasif you stopped a dead corner, then sure :)15:38
sean-k-mooneyi.e. i did not remove the mks console server or any of the extra specs validation ectra15:38
bauzasspotted*15:38
sean-k-mooneybut want to hear how people want that to be done15:38
bauzasyeah, I thought it was API-related15:38
sean-k-mooneythe currnt patch is not api related15:39
bauzaswe have a solid experience on API stability while we remove backends15:39
sean-k-mooneyjust the virt driver and remvoe teh virt dirver spcifric cofnig options and docs.15:39
sean-k-mooneyi belive gman has also already done hyperv and has the same open15:39
sean-k-mooneyi.e. waht to do with hyperv specific api aspect15:39
sean-k-mooneyso i wanted to group does disucssions togetehr15:40
sean-k-mooneysince hte approch should likely be the same15:40
bauzassounds a good idea to me15:41
bauzasthanks for catching it15:41
sean-k-mooneydo we have the ptg etherpad yet15:41
bauzasyep15:42
bauzasoh but you weren't in the last meeting, sec15:42
sean-k-mooneyya still catching up form PTO15:42
bauzasnp15:42
bauzashttps://etherpad.opendev.org/p/nova-caracal-ptg15:43
bauzas(didn't made any huge effort for the naming)15:43
sean-k-mooneycool ill add something to that then and you cna feel frre to rearragne things15:44
bauzascool15:45
opendevreviewSylvain Bauza proposed openstack/nova master: doc: clarify that pre-commit is optional  https://review.opendev.org/c/openstack/nova/+/89705715:53
stephenfinsean-k-mooney: I wonder if we could start setting and exposing ACPI index for network devices, as a consistent identifier that will be respected (unless PCI device addresses) https://libvirt.org/formatdomain.html#network-interfaces16:08
sean-k-mooneyhum let me read16:08
sean-k-mooneybut maybe16:08
sean-k-mooney"""Since 7.3.0, one can set the ACPI index against network interfaces. With some operating systems (eg Linux with systemd), the ACPI index is used to provide network interface device naming, that is stable across changes in PCI addresses assigned to the device. This value is required to be unique across all devices and be between 1 and (16*1024-1)."""16:09
sean-k-mooneyinteresting so ya maybe16:09
sean-k-mooneystephenfin: of the top of my head i dont see why not16:10
sean-k-mooneywell hum16:11
sean-k-mooneythere might be some issues with that16:11
sean-k-mooneyso the problem iw going to be how to corralate it with the port 16:11
sean-k-mooneyso we willl likely need to store it in the virtural interfaces table16:11
sean-k-mooneyso that its accesable to the api or store it in the binding_profile on the neutorn port16:12
sean-k-mooneyok so testing the acpi index theory  <acpi index="4"/> is accepted in the xml16:52
sean-k-mooneyand the interface name becomes eno416:52
sean-k-mooneyso that is a good sigh16:52
sean-k-mooneysean@upstream-devstack:~$ cat /sys/class/net/eno4/device/acpi_index 16:53
sean-k-mooney416:53
sean-k-mooneystephenfin: ^ so ya that is trivially discoverable16:53
stephenfin \o/16:53
stephenfinnoice16:53
sean-k-mooneyso if we were to add that to the metadata that would work16:53
sean-k-mooneyim going to make it 42 just to be sure it actully is working the way i think16:54
sean-k-mooneysean@upstream-devstack:~$ cat /sys/class/net/eno42/device/acpi_index 16:55
sean-k-mooney4216:55
sean-k-mooneyyep so it will result in a stable device name as well as the docs suggest16:55
opendevreviewDan Smith proposed openstack/nova master: Sanity check that new hosts have no instances  https://review.opendev.org/c/openstack/nova/+/89707618:22
opendevreviewDan Smith proposed openstack/nova master: Sanity check that new hosts have no instances  https://review.opendev.org/c/openstack/nova/+/89707619:35
opendevreviewsean mooney proposed openstack/nova master: remove deprecated vmware virt driver  https://review.opendev.org/c/openstack/nova/+/89701720:38
opendevreviewsean mooney proposed openstack/nova master: fix sphinx-lint issues in releasenotes  https://review.opendev.org/c/openstack/nova/+/89708722:04
opendevreviewsean mooney proposed openstack/nova master: fix sphinx-lint issues in api guide  https://review.opendev.org/c/openstack/nova/+/89708822:04
opendevreviewsean mooney proposed openstack/nova master: fix sphinx-lint errors in docs and add ci  https://review.opendev.org/c/openstack/nova/+/89708922:04
opendevreviewsean mooney proposed openstack/nova master: [codespell] start fixing all the typos  https://review.opendev.org/c/openstack/nova/+/89709223:54

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!