Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-06-10
10:43:09 bauzas stephenfin: https://review.opendev.org/#/c/733668/2 got a -1 because of PEP484 usage be py3 specific
10:43:35 gibi stephenfin: thanks for the reno
10:44:17 sean-k-mooney bauzas: we are allowed to use py3 only syntax and feature on master
10:44:25 bauzas I know
10:44:28 stephenfin I was about to say :)
10:44:43 sean-k-mooney bauzas: and this is not enforce excpetion type it adding them
10:44:49 stephenfin personally, I think this patch is too invasive to backport
10:44:51 bauzas but then if we backport, we'll get linters shouting at us with no idea why
10:45:22 sean-k-mooney if we backport we need to drop the type info
10:45:36 stephenfin I think we're just going to have to live with that. I'll just drop the type info if I went back past stable/ussuri
10:45:40 sean-k-mooney we were holding of on adding it so we did not need to use the comment syntax
10:45:50 stephenfin yeah, exactly
10:45:58 bauzas stephenfin: then, just amend the commit msg and we're all good
10:46:25 stephenfin Sure. What should I add?
10:46:44 bauzas I don't want to refrain ourselves to use py3 styles, but I rather just want us to take 5 mins before writing it and consider backportability of such things
10:46:55 bauzas stephenfin: just tell your decision
10:46:55 sean-k-mooney it took me a long time to be ok with the type on the right after all the c/c++/java/c# i wrote before learning python but im slowly gettin gused to it
10:47:11 sean-k-mooney it seam like most lanaguges are converging on that order now
10:47:15 bauzas option A/ this bugfix isn't intended to be backported
10:47:34 bauzas option B/ if people feel brave enough to backport, there will be a necessary change
10:47:49 bauzas in other words, document your code :)
10:48:03 sean-k-mooney bauzas: so im ok with b if iall it is is removing :<Type>
10:48:03 stephenfin Hmm, I _could_ but it does feel like stating the obvious
10:48:19 bauzas sean-k-mooney: me too, just my point
10:48:28 bauzas again, it's just a static check AFAIK
10:48:30 sean-k-mooney i dont think that need to be in a comit or called out anywhere in the chagne
10:48:45 sean-k-mooney bauzas: well its not even a staitc check
10:48:51 sean-k-mooney the py2 jobs will fail
10:49:00 sean-k-mooney it wont parse correctly
10:49:02 stephenfin We'd saying if you use Py3 syntax and your commit message has "Closes-Bug" in it, add this boilerplate
10:49:32 sean-k-mooney stephenfin: i would really like to avoid that
10:49:38 stephenfin I mean, we don't say "this bugfix has o.vo implications so you're not allowed backport it" in a commit
10:49:40 sean-k-mooney we coudld but i dont see the point
10:50:11 bauzas sean-k-mooney: the point is that I'm afraid stable core reviewers could miss such tiny bits that would break
10:50:11 sean-k-mooney stephenfin: bauzas we could update the contibutor guide or somehting to call it out i guess
10:50:23 sean-k-mooney bauzas: but it would not pass ci
10:50:31 sean-k-mooney so it wont get merged at least
10:50:32 bauzas fortunately, in this specific case, agreed, CI won't run
10:50:37 stephenfin Fortunately, as sean-k-mooney points out, this will raise a SyntaxError on python 2.7
10:50:38 sean-k-mooney but they might not know why
10:50:56 stephenfin heh, yeah 0:)
10:51:00 bauzas this
10:51:10 bauzas honestly, I don't want to bikeshed
10:51:10 stephenfin how a contributor guide change?
10:51:12 stephenfin *about
10:51:29 bauzas you know what ? I'll just approve the change
10:51:31 stephenfin Do we have a backporting section in that?
10:51:44 bauzas and then people who backport should just see my comment in the review
10:51:50 stephenfin \o/ WFM too :)
10:52:00 bauzas if they don't, then I'll kill them
10:52:06 bauzas seriously
10:52:50 bauzas stephenfin: but for a second, we unnecesseraly added complexity to the existing code by adding a linter check
10:53:14 bauzas I could have -1d it by saying 'sorry dude, unrelated to the change'
10:53:37 bauzas stephenfin: so, please refrain yourself to add more than what's necessary ;) :D
10:53:46 stephenfin ooooorrr, we're reducing complexity by removing the need to worry about types
10:53:53 sean-k-mooney bauzas: i could argure the opiste
10:54:07 stephenfin let the compute do the work for you
10:54:09 sean-k-mooney if you dont add it and added it in a seperate change that is adding uneesisary overhead in revew
10:54:11 stephenfin *computer
10:54:25 stephenfin agreed
10:54:39 stephenfin if I wasn't changing the function signature, this would be unrelated
10:54:47 stephenfin but because I am, this is a fair change
10:54:54 sean-k-mooney anyway stephen are you going to chekc the hugepage/file backed memeory thing or will i doit
10:55:23 stephenfin sean-k-mooney: If you've time, go for it. I'm going to lunch shortly as I'm in meetings most of this afternoon :(
10:55:28 stephenfin If not, I'll aim to do it before EOD
10:55:46 sean-k-mooney well i want to understand this so that i can figure out how this relates to numa in placmenet
10:56:03 sean-k-mooney so ill try and doit shortly
10:57:06 sean-k-mooney file backed memroy will have to be reported on the ROOT RP
10:57:18 sean-k-mooney since it has no real numa affintiy
10:59:44 gibi I think we can trust the stable cores to see if a code is not py2 compatible
11:00:22 sean-k-mooney the fact that we are not using six anymore will liekly cause more issue then type hints
11:00:34 gibi also if there is such code that valid for py2 and py3 but behaves differently, then during the master review we have to make sure we add enough unit test coverage that will break when the patch is blindy backported to py2
11:00:54 sean-k-mooney yep
11:01:15 gibi in this particular case the interpreter catches the problem
11:01:47 bauzas gibi: sean-k-mooney: what I'm saying is that we somehow opted into thinking more about what we write and what we merge on master now that the ship has sailed
11:01:53 bauzas that's it.
11:01:58 sean-k-mooney list vs iterator/view will be one of the main one for like dict.keys()
11:02:10 stephenfin and unicode/bytes
11:02:20 bauzas previously, we had no brain knobs to do as we were supporting boh
11:02:22 bauzas both*
11:02:22 sean-k-mooney in that case its not as bad
11:02:29 sean-k-mooney as we are sticter in py3
11:02:36 gibi bauzas: sure, we should be aware, but at the same time we should not limit ourselves to write better code on master
11:02:36 sean-k-mooney but yes
11:02:47 bauzas I don't disagree
11:03:15 bauzas I'm just saying we need to ponderate the will of writing nice code with the pragmatism of running such code on different stable releases
11:03:54 bauzas and from someone who sometimes has to backport things down to Newton, I'm thinking about my job for the next 3 years
11:04:13 bauzas if I can ease myself, I'll certainly do
11:06:32 sean-k-mooney stephenfin: so looking at http://paste.openstack.org/show/794558/
11:06:40 sean-k-mooney stephenfin: i think we are good on the numa front
11:09:08 sean-k-mooney stephenfin: this is the relevent virsh output http://paste.openstack.org/show/794560/
11:09:40 sean-k-mooney so libvirt sees the hugepage i have allocated but we are not reporting any hugepages in the host numa_toplogy blob
11:11:50 sean-k-mooney gibi: by the way inital_ram_allocation_ratio still default to 1.5 :( i think we just changed it to 1 in ooo
11:12:29 sean-k-mooney we said we chagned it to 1.0 in the ptg but we did not
11:12:44 gibi sean-k-mooney: yeah, we mentioned 1.0 but I don't remember the context
11:13:03 gibi I do know that the product I helping downstream set it 1.0 directly since Icehose or Mitaka
11:13:19 sean-k-mooney ya and windriver did too
11:13:55 sean-k-mooney maybe we should actully chage the upstream default to 1 but we would have to be very vocal and proably add an upgrade check if we did
11:15:01 sean-k-mooney 16 for our cpus is also nuts as a defalt. anything over about 4 tends to cause issue when the system is loaded
11:15:58 gibi sean-k-mooney: agree about vocalization, but I don't think we should spend too much effort changing this default.

Earlier   Later