| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-02-18 | |||
| 13:40:20 | kashyap | I just feel it is bloated to add needlessly extra options | |
| 13:40:31 | sean-k-mooney | kashyap: im pretty sure other cores have asked you in the passed to have the two config options | |
| 13:40:31 | kashyap | sean-k-mooney: I know you were in the "+/- camp" as you put it | |
| 13:40:35 | kashyap | No | |
| 13:40:42 | sean-k-mooney | i think i rememebr who they were too | |
| 13:40:49 | kashyap | sean-k-mooney: The patch is only posted now. There was no patch before | |
| 13:40:50 | sean-k-mooney | i just dont want to name drop in case im wrong | |
| 13:40:56 | kashyap | You're imagining the conversation. Really. | |
| 13:41:04 | sean-k-mooney | kashyap: we have discussed this in person at a desing summit before | |
| 13:41:47 | kashyap | Oh, well. I was not there; or don't remember the details. | |
| 13:42:35 | kashyap | It is simply overkill; and is not just worth it. What is there is clear, and intuitive. That suffices. | |
| 14:03:45 | bauzas | stephenfin: working on the new rev | |
| 14:06:02 | bauzas | stephenfin: could we discuss on https://review.opendev.org/c/openstack/nova/+/773976/7/nova/exception.py ? | |
| 14:10:33 | stephenfin | bauzas: sure, what's up? | |
| 14:10:45 | stephenfin | did you see my comment on the next patch also? | |
| 14:12:15 | bauzas | stephenfin: just uploaded my comments | |
| 14:13:35 | sean-k-mooney | ah here https://review.opendev.org/c/openstack/nova/+/773976/7/nova/scheduler/utils.py#1370 | |
| 14:13:39 | sean-k-mooney | ys its used in both | |
| 14:14:53 | sean-k-mooney | thats called form the request filter in the next change right | |
| 14:15:23 | sean-k-mooney | ya here https://review.opendev.org/c/openstack/nova/+/749068/17/nova/scheduler/request_filter.py#341 | |
| 14:15:30 | bauzas | sean-k-mooney: yup | |
| 14:15:41 | bauzas | honestly, we should just supporting this exception | |
| 14:15:47 | bauzas | for all of the issues | |
| 14:15:58 | bauzas | we could have other exceptions, like for looking at the port | |
| 14:16:19 | bauzas | but for getting a NoValidHost, just the above should be needed | |
| 14:17:53 | bauzas | anyway, stephenfin provided me a good concern for https://review.opendev.org/c/openstack/nova/+/749068/17/nova/tests/fixtures.py so I'll provide a new revision, but in case we want to dicuss about exceptions here, maybe let's agree on providing a new FUP then | |
| 14:18:32 | stephenfin | bauzas: hmm, so we do | |
| 14:18:59 | stephenfin | I hate to say it but could we use two exceptions? /o\ | |
| 14:19:22 | stephenfin | I know it's sort of bikeshedding, but I'm uncomfortable with using non-generic exceptions across multiple "services"# | |
| 14:20:02 | stephenfin | with the obvious caveat that if everyone else disagrees with me or thinks it's not an option, I can go along with the majority | |
| 14:22:11 | bauzas | well, I can see your concerns | |
| 14:22:26 | bauzas | let me try to first upload the new series and discuss about it later for a FUP | |
| 14:22:47 | stephenfin | sounds good to me | |
| 14:42:12 | bauzas | stephenfin: huh, /me is a sad panda again due to mypy https://review.opendev.org/c/openstack/nova/+/773976/7/nova/network/neutron.py#3533 | |
| 14:42:26 | bauzas | because of static type checking, we need to change code | |
| 14:42:51 | bauzas | it's like when we want to tell about objects and we need to avoid circular imports | |
| 14:43:06 | bauzas | sounds to me that mypy is strictier than it should and prevents us good faith | |
| 14:43:42 | bauzas | any static type checker shouldn't be invasive | |
| 14:44:53 | bauzas | so, I'll explicitly return None as I stepped into this 'shit' (with the left foot hopefully), but that won't convince me of the mypy opportunity | |
| 14:45:37 | bauzas | and if I was grumpy (which i was horribly last days and I apologize), I could just remove all my annotations from the code I wrote | |
| 14:45:47 | bauzas | but I'll continue the game | |
| 14:47:14 | bauzas | at least, having static type tool developers being pedantic and opionated about what python should be seems to be fragile for the very least | |
| 14:47:14 | openstackgerrit | Lucas Alvares Gomes proposed openstack/nova master: [OVN] Adapt the live-migration job scripts to work with OVN https://review.opendev.org/c/openstack/nova/+/776419 | |
| 14:47:24 | gibi | bauzas: I felt the same when I got introduced to this return None change. It is a pain. But it is a one time pain and I believe the explicit types will give us gains when reading and changing the code later | |
| 14:47:58 | bauzas | again, not convinced from the experience I have of various linters | |
| 14:48:15 | bauzas | and again, python is all duck typing | |
| 14:48:39 | bauzas | having developers convinced of the opposite seems terrible to me frankly | |
| 14:49:10 | bauzas | the fact is, OpenStack is made with python, OK | |
| 14:49:26 | bauzas | but it's just a language | |
| 14:49:54 | bauzas | and we shouldn't be doing pythonesque things because of python, but rather because we want it | |
| 14:50:51 | sean-k-mooney | bauzas: i think the different is that we disagree on what improves the code | |
| 14:51:51 | bauzas | being explicit on the fact that return has to return None doesn't sound to me a major improvement :) | |
| 14:52:37 | bauzas | and a linter never proved me enough confidence in the fact we would write better code | |
| 14:52:51 | bauzas | better code is written by people who better understand code | |
| 14:52:56 | sean-k-mooney | it depens how far you take it. if you look at c++ for example being able to annotate funcitons with nodiscard is powerful | |
| 14:53:18 | bauzas | and code reviews certainly help more than any linter | |
| 14:53:21 | sean-k-mooney | e.g. it allows you to state if you dont at least store and preferebaly check the retrun value then its a logic error | |
| 14:53:39 | bauzas | like, the best comments from stephenfin would never be caught by a linter | |
| 14:53:57 | bauzas | and I feel we are distracting ourselves | |
| 14:54:17 | sean-k-mooney | true but i think type hint aid understanding. we dont have to use them but i think we would have better code if we did | |
| 14:54:32 | bauzas | giving us false confidence that we will improve our internal APIs | |
| 14:55:11 | bauzas | fwiw, I did a lot of programming with strong typed languages | |
| 14:55:12 | sean-k-mooney | well i have seen too many bugs cause by using incorrect data in our tests to think that if we enforced type better it would not happen | |
| 14:55:25 | bauzas | and I never saw this being a benefit for distributed programming | |
| 14:55:41 | sean-k-mooney | python is a stongly typed language | |
| 14:55:46 | bauzas | Java pursues the idea that annotations help microservices | |
| 14:55:48 | sean-k-mooney | its just not staticly typed | |
| 14:56:12 | bauzas | but trust me, you can write terrible microservices in Java despite using annotations | |
| 14:56:23 | sean-k-mooney | of coruce you can | |
| 14:56:32 | sean-k-mooney | its a tool like any other | |
| 14:56:37 | bauzas | right | |
| 14:56:42 | sean-k-mooney | you can always missuse a tool if you dont understand it | |
| 14:57:05 | bauzas | but if a tool (or a process), instead of easing my work, requires me to further work, this doesn't sound to me then right | |
| 14:57:46 | sean-k-mooney | well personly i always found the parms in docs sting unhelpfull and a hinderance to future work | |
| 14:57:57 | bauzas | and here, explicitly stating "return None" in order to get my code merged doesn't sound to me great at most | |
| 14:58:02 | sean-k-mooney | they get out of data and you have to check it | |
| 14:58:38 | sean-k-mooney | well you are relying on an implict convertion to None otherwise | |
| 14:58:47 | sean-k-mooney | if you just do return | |
| 14:58:52 | bauzas | sean-k-mooney: they get out of data by lack of solid reviews | |
| 14:58:53 | sean-k-mooney | return None is more explcit | |
| 14:59:09 | sean-k-mooney | not eveyone know if you do return it accutly become the same a return None | |
| 14:59:49 | bauzas | again, that's why we have Gerrit | |
| 15:00:04 | bauzas | humans are the best linters by far | |
| 15:00:30 | sean-k-mooney | im not sure this is getting us anywhere productive | |
| 15:01:06 | sean-k-mooney | and since im currently in a low to moderate amount of pain im goignto try an get some work done sorry | |
| 15:01:21 | gibi | bauzas: return None is price to gain every benefit of mypy, it is not return None that directly helps you | |
| 15:03:59 | gibi | return None enables mypy to helps us :) | |
| 15:04:16 | bauzas | gibi: my other concern is that we currently make mypy fencing our gate | |
| 15:04:29 | sean-k-mooney | bauzas: we dont | |
| 15:04:39 | bauzas | sean-k-mooney: really ? | |
| 15:04:45 | sean-k-mooney | its not enforced by ci | |
| 15:04:58 | sean-k-mooney | unless we put it in the pep8 job | |
| 15:05:04 | sean-k-mooney | but i did not think we did | |
| 15:05:08 | bauzas | then, can I leave "return" without explicitely following it by "None" ? | |
| 15:05:20 | sean-k-mooney | let me check | |
| 15:05:45 | bauzas | in the past, we decided which PEPs we were following | |
| 15:06:08 | sean-k-mooney | oh sorry ya we do https://github.com/openstack/nova/blob/master/tox.ini#L57 | |
| 15:06:14 | sean-k-mooney | but only on the files we have enabled | |
| 15:06:20 | sean-k-mooney | thats why im miss remembering | |
| 15:06:35 | bauzas | right, my point | |