Friday, 2023-09-22

opendevreviewOpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata  https://review.opendev.org/c/openstack/nova/+/89617503:20
bauzasfyi, folks, did the launchpad usual janitory for creating a new series and milestones on both placement and nova : https://launchpad.net/placement/2024.1 and https://launchpad.net/nova/2024.109:07
bauzasas you can see, I stopped using the release name to mock what we already have in our specs repo09:08
opendevreviewMerged openstack/nova master: Imported Translations from Zanata  https://review.opendev.org/c/openstack/nova/+/89617509:36
stephenfinbauzas: Can we merge the outstanding sqlalchemy 2.0 stuff now?10:11
stephenfinactually, it's sean-k-mooney I should be asking. Sean, could you look at https://review.opendev.org/c/openstack/nova/+/860829/ again?10:12
sean-k-mooneyyes please10:13
stephenfinthough I am going to respin https://review.opendev.org/c/openstack/nova/+/886230 to address those regex warnings10:13
sean-k-mooneywhat about https://review.opendev.org/c/openstack/nova/+/89463510:15
opendevreviewStephen Finucane proposed openstack/nova master: Add job to test with SQLAlchemy master (2.x)  https://review.opendev.org/c/openstack/nova/+/88623010:16
stephenfinsean-k-mooney, bauzas: ^ fixed to address the RE2 warning from zuul. It's otherwise unchanged.10:16
stephenfinsean-k-mooney: I'll clean that last patch up today10:16
stephenfinactually, intermittent failure. That looks good to me now but I think bauzas wanted a little more proof that the relationships weren't used anywhere10:17
stephenfintrying to figure out how I can prove that besides code inspection (I tried that. Too slow)10:17
sean-k-mooneyi didnt think it was related and likely just needed a retry10:24
sean-k-mooneyi was more wondifning if you still wanted to proceed with that patch10:24
sean-k-mooneythe first two in teh series are +2w ill review the ci patch shortly10:24
opendevreviewFelix Huettner proposed openstack/nova master: remove calls used for nova-network  https://review.opendev.org/c/openstack/nova/+/89622111:00
opendevreviewStephen Finucane proposed openstack/nova master: doc: Remove crud from conf.py file  https://review.opendev.org/c/openstack/nova/+/89622411:55
opendevreviewStephen Finucane proposed openstack/nova master: Apply latest version of autopep8  https://review.opendev.org/c/openstack/nova/+/89622511:55
opendevreviewStephen Finucane proposed openstack/nova master: pre-commit: Update plugin versions  https://review.opendev.org/c/openstack/nova/+/89622611:55
opendevreviewStephen Finucane proposed openstack/nova master: tox: Use pre-commit for pep8 target  https://review.opendev.org/c/openstack/nova/+/89622711:55
opendevreviewStephen Finucane proposed openstack/nova master: pre-commit: Add mypy  https://review.opendev.org/c/openstack/nova/+/89622811:55
opendevreviewStephen Finucane proposed openstack/nova master: Blacken codebase  https://review.opendev.org/c/openstack/nova/+/89622911:55
opendevreviewStephen Finucane proposed openstack/nova master: pre-commit: Integrate black  https://review.opendev.org/c/openstack/nova/+/89623011:55
opendevreviewStephen Finucane proposed openstack/nova master: Ignore black change  https://review.opendev.org/c/openstack/nova/+/89623111:55
stephenfinsean-k-mooney: ^11:55
sean-k-mooney:)11:55
stephenfinThat's what I was talking about. We just disable E501 because it's not worth the hassle of manually fixing the dozen or so exceptions11:55
stephenfinIt also avoids questions from contributors less familiar with the tooling. We can just tell them "run this and you'll get no style-related questions"11:56
sean-k-mooneyya if other are ok with that then i would be infavor of the general useablity improvement11:56
stephenfin(as opposed to "run this and then manually fix things if you still get errors")11:56
sean-k-mooneyalthough the eiarlier patches i htink should be done regradless11:56
sean-k-mooneyso ill start reveiwign those11:56
stephenfinyup11:56
stephenfinI put those at the front since they should be non-controversial11:57
fricklerwhile you're doing linting stuff, please also check https://review.opendev.org/c/openstack/nova/+/874517 , it seems to keep failing on unrelated issues12:04
sean-k-mooneysome of that might get adresses by these patches as well but sure12:04
sean-k-mooneyya the bump to the new autopep8 fixes many of the  hacking issues12:05
stephenfinoh yeah, there's quite a bit of overlap there. The only thing mine are missing is the switch from printf style formatting (%) to use of string.format. I wonder why I didn't have to worry about that?12:05
sean-k-mooneyits not a pep8 requriement12:06
sean-k-mooneyand i would honestly prefer to go to fstring if we were updating them12:06
fricklero.k., if that's no longer needed, just updating https://review.opendev.org/c/openstack/hacking/+/874516 would also be fine to me12:06
fricklerand once that's done we can do the next flake8 bump in hacking ;)12:06
bauzasstephenfin: sorry was outside sweating12:07
bauzasstephenfin: sure, if we want to merge your series, I prefer to do this on the starting Caracal :)12:07
sean-k-mooneyfrickler: so stephen's partches are largely generated form tools12:11
stephenfinbauzas: tbf, we say that every cycle and then we forget to look at it until the end of the cycle (see all discussions on the outstanding sqlalchemy 2.x work from last week) 0:)12:11
bauzasstephenfin: that's why this is fresh in my mind12:11
stephenfinthe changes (except for the black one) are minimal and we've cut the branch, can we review now and if we need to fast revert, we can12:11
sean-k-mooneyso we will just proceed with the hacking fix patch and then regeenrate the ones that are still needed form there pre-commit series12:11
stephenfinit'd likely be a win for you too: if we operated this way, I'd have stopped annoying you about sqlalchemy 2.x ~6 months ago XD12:12
stephenfinsean-k-mooney: wfm12:12
sean-k-mooneyRC1 and stable/2023.2 have been done now12:13
sean-k-mooneyso i think now is a perfect time to do linting updates to be honest12:13
bauzascan we try to not mix linter changes *and* SQLA things at the same time ?12:16
sean-k-mooneythey are two diffent serise12:16
bauzasI know12:16
bauzasbut you know my backportability concerns about any linting change12:16
sean-k-mooneyyep and you knwo i disagree with that stance. that said the early one should have no impact12:17
bauzasthose need to be reasonabily needing a reason, because it makes any other backports a pain12:17
bauzaswe suffered a lot with functional tests refactoring, but this was kinda understansable12:17
bauzassince it was improving tests reusability12:18
stephenfinbauzas: out of curiosity, could you point to a recent case where a backport was impeded because of one of these linting change?12:18
sean-k-mooneyhonetsly not automating this makes it harder for use to maintain nova12:18
sean-k-mooneyand it makes it harder for new peopel to continbue. with the reduced upstream comunity i think it would be in our intereste to embrance this automation where we can12:19
bauzasstephenfin: I don't have any examples of the linting changes in mind, but be sure we faced some problems for other bits12:19
bauzassean-k-mooney: I understand that point for refactoring things12:19
bauzassean-k-mooney: but for linting, I seriously argue12:19
sean-k-mooneyyep and ill happy argue back12:20
bauzascode style modifications being even less convincing to me12:20
sean-k-mooneyhonestly i have spend enough time this cycle on sytle issues in code review12:20
sean-k-mooneythat if this is the only thing we got done in caracal i woudl consider it a success12:21
bauzasI know it's Friday, but is this a reason to argue now ?12:21
sean-k-mooneyi mean its friday what else would we do12:22
stephenfin:)12:22
sean-k-mooneygo home early and realx12:22
bauzasI already sweated and now I have endorphins12:24
bauzasnot sure my stress level should jump, not good for my heart12:24
sean-k-mooneyif the wether stays ok i might go for a bike ride later12:24
sean-k-mooneyalthogh its a bit humid out so we will see12:24
bauzasseriously, I'm half-kidding, but feel free to put code styling and autoformatters on the PTG agenda12:24
opendevreviewMerged openstack/nova master: db: Replace use of backref  https://review.opendev.org/c/openstack/nova/+/86082912:25
bauzasnot sure my opinion will evolve but I'm still open to discuss about it12:25
bauzasI just don't think the benefits overcome the pain12:25
sean-k-mooneyhonestly the pain of not having them makes me somethimes condier if i wnat to keep manintianing nova12:26
bauzasand about exceprts of code changes that pulled extra complexity into backports, I can do some homework and find some series12:26
sean-k-mooneybauzas: i would happly backport the autofromating if that woudl make you feel beeter12:26
bauzasnot out of my head at first sight unfortunately, but there are experiences for sure12:26
bauzassean-k-mooney: down to Train ? :)12:26
sean-k-mooneysure12:26
sean-k-mooneythe patches would be generated by the tool12:27
sean-k-mooneywe woudl jsut have to review it once and its done for ever12:27
sean-k-mooneyif that is your main objection its sovleabel. but im also oke with fixing this when backporting12:27
bauzasthat's indeed my main objection, beyond just a feeling12:28
sean-k-mooneyas you know i have been working in both go and python lately12:28
bauzasI'm personnally against such things because I think it negates the stance of writing, but this is a personal opinion and absolutely not arguable12:29
sean-k-mooneyand ansible... but it has renifoced that  feelting that auto liniting is good12:29
sean-k-mooneyand auto formating is both good for the reviewer and author12:29
bauzasI can't wait for a ChatGPT bot on Visual Code12:30
bauzasVS Code*12:30
stephenfinhttps://marketplace.visualstudio.com/items?itemName=GitHub.copilotvs12:30
sean-k-mooneyhehe i already use code poilte ot help writhe my comments12:30
* bauzas facepalms12:31
sean-k-mooneyits why they have had less typos lately12:31
bauzasand yeah, I heard about GH CoPilot12:31
* bauzas is just about to give up when he reads this12:31
sean-k-mooneyit daydreams a bit so unless you know the code base i would not recomend it to new commers12:32
sean-k-mooneyi have mostly been using it of go/ansible but i find i have to check its work and rewite it but its often close to what i would write12:33
bauzasthat's the problem with those AI tools12:33
bauzas95% of the job is perfectly done12:33
bauzasand then the rest is errorprone12:33
sean-k-mooneyyep and that last 5% is where being a software engineer comes in12:33
bauzasI'm not sure my productivity will raise because of a bot writing my code12:34
sean-k-mooneyusing your expirnce ot understand its probmpts and correct them12:34
bauzasmost of the time it takes for me to write a feature is : 1/ identify the gaps, 2/ come with a proposal, 3/ let the community agree on the proposal, 4/ write that 12:35
bauzasthe 4/ isn't exactly hard to do12:35
sean-k-mooneysure and most of the process of adding a feature into nova is code review12:36
sean-k-mooneynot wirting the code in the first place12:36
bauzashonestly, I feel the entry barrier on Nova isn't because we're not having black style and autoformatters12:37
bauzasbut rather because it's a very large distributed software contributed by many in different times12:38
bauzasand I don't think autoformatters will lower this barrier12:38
sean-k-mooneysure but that misses the point12:38
bauzaswhich is that people can autotab their brackets ?12:39
sean-k-mooney no12:40
stephenfinit's a quality of life improvement12:40
sean-k-mooneyyep12:40
stephenfinbeing a surgeon is hard: working in a clean operating theatre makes it slightly easier12:40
bauzasI see myself more like a butcher :D12:41
bauzasanyway, I stink and I need to get a shower12:41
stephenfinthen black is a sharp knife12:41
stephenfin;)12:41
stephenfinor rather an automatic knife sharpener12:41
sean-k-mooneyi was going to play the frech card and say its a Charcuterie12:41
bauzasstephenfin: if we continue the analogy, people can get hurt by a sharp knife if they don't know how to use it :)12:42
sean-k-mooneywhich is another reason my analogy is better :)12:42
* bauzas disappears for 10 mins12:42
sean-k-mooneybe a charcutier not a butcher of our lovely codebase :)12:43
sean-k-mooneystephenfin: since your about can you look at https://review.opendev.org/c/openstack/nova/+/89565512:55
sean-k-mooneyonce thats merged i can propsoe the revert adn ping it to rodolfo and co so we can renable it once neutron says the test is fixed for ovn12:56
stephenfinack, done12:56
sean-k-mooneythat test is one of the thing that cased the hackign patch to fail12:56
sean-k-mooneythe other failure was volume detach related12:57
bauzasfwiw, starting to look at the rest of sql14 series13:05
opendevreviewFelix Huettner proposed openstack/nova master: ignore errors when deleting volume attachments  https://review.opendev.org/c/openstack/nova/+/89623513:09
bauzasokay, this is Friday, I'm not a better and decent position to start a debate https://luminousmen.com/post/my-unpopular-opinion-about-black-code-formatter13:18
bauzasnow*13:18
sean-k-mooney for what its worht a number of opentack proejct have arelady adopted black13:18
bauzasthat doesn't make the point valid :)13:19
sean-k-mooneywe already have autopep8 which was a compomise13:19
bauzasyup, and which doesn't force your code to change, right?13:19
bauzasjust because a ending bracket has to be on a separate line13:20
sean-k-mooneyit only fixes code if it not pep8 complient13:20
sean-k-mooneyand it undersands the things we disable in tox.ini13:20
bauzasblack is *very* noisy in terms of code change since it can't adapt to the existing code style13:20
sean-k-mooneyi.e. it reads https://github.com/openstack/nova/blob/master/tox.ini#L30713:21
bauzasand it forces us to do the way blacks thinks being the best13:21
sean-k-mooneyi think that is better then use disagreeing on what is the best style13:22
bauzasbtw. this twitter post which is replied by Guido is absolutly gold13:22
sean-k-mooneybauzas: by the way i actully dont really care what the tool is provide we have one that does it 13:23
bauzasit shows the mindset of many developers : in order to follow *best* practices of coding, it says "install X and Y"13:23
bauzasmy head explodes when I read this13:23
bauzasI thought for 2 decades that coding best practices were about code patterns and reusability13:23
sean-k-mooneypartly but we are not talking about best praticies13:24
sean-k-mooneyif you think we are then again your are missing the point13:24
bauzassean-k-mooney: read my link above, you'll see this tweet :)13:24
sean-k-mooneyi have seen those before13:24
bauzasouch https://black.readthedocs.io/en/stable/guides/introducing_black_to_your_project.html#avoiding-ruining-git-blame13:28
bauzassean-k-mooney: just in order to move on black and because I want to exactly understand the implications, do you know how the other projects manage the git blame mangling thing ?13:47
sean-k-mooneyi belvie you can annotate the commit to be ignored for blame13:48
sean-k-mooneyand it will then not be seein in github or locally by default13:48
sean-k-mooneybauzas: which stephen did there https://review.opendev.org/c/openstack/nova/+/89623113:49
bauzasyeah, that's what black is saying on their docs13:49
bauzasokay, then that's planned, gtk13:49
sean-k-mooneyyep its definetly a nice ux improvment ot have that13:56
sean-k-mooneyalthoguh i woudl also take blame working in github at all on the libvirt driver...13:57
*** EugenMayer4401 is now known as EugenMayer44016:04
opendevreviewMerged openstack/nova master: Fix pep8 errors with new hacking  https://review.opendev.org/c/openstack/nova/+/87451717:34

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