| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-27 | |||
| 10:18:19 | aspiers | bauzas: I was just looking at fixing some of the issues in sean-k-mooney's patch you just reviewed https://review.opendev.org/#/c/666914/ | |
| 10:18:32 | bauzas | cool | |
| 10:18:50 | aspiers | although I'm not sure if I'd be treading on his toes | |
| 10:19:01 | bauzas | well, sean is there :) | |
| 10:19:12 | aspiers | https://storage.gra1.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/logs_14/666914/11/check/openstack-tox-py27/ee133dd/testr_results.html.gz shows an ordering error which is probably the dict.values issue you mention | |
| 10:19:19 | aspiers | only on py27 not py3 | |
| 10:19:34 | aspiers | I'm not sure if he's around today | |
| 10:20:39 | sean-k-mooney | bauzas: yep i know values change between py27 and py3X | |
| 10:20:50 | sean-k-mooney | i normally would use six.iteritems | |
| 10:21:03 | sean-k-mooney | but aspiers and stephenfin prefer that i dont | |
| 10:21:11 | openstackgerrit | Gorka Eguileor proposed openstack/nova master: Use os-brick locking for volume attach and detach https://review.opendev.org/614190 | |
| 10:21:18 | aspiers | sean-k-mooney: I don't personally care but it seems that everyone else does | |
| 10:21:28 | bauzas | why so ? | |
| 10:21:37 | sean-k-mooney | aspiers: i dont like use writing slow code by default | |
| 10:21:58 | bauzas | wait, six.itervalues() isn't exactly slow | |
| 10:22:14 | sean-k-mooney | no thats what i prefer to use | |
| 10:22:16 | sean-k-mooney | for values | |
| 10:22:22 | sean-k-mooney | or iteritems for items | |
| 10:22:23 | stephenfin | I only care insofar as it generally doesn't make any difference and I'd rather one less 'six' thing to clean up when we move to Python 3 only | |
| 10:22:38 | sean-k-mooney | stephenfin: in aggreate it does | |
| 10:22:46 | bauzas | https://github.com/benjaminp/six/blob/master/six.py#L584 | |
| 10:23:02 | stephenfin | on python 2, yes | |
| 10:23:04 | bauzas | that's what does six.itervalues | |
| 10:23:05 | aspiers | I'm with stephenfin on this. The majority of cases we are talking about involve like 3 elements | |
| 10:23:15 | bauzas | stephenfin: of course | |
| 10:23:15 | stephenfin | but OSP 16 will be deployed with Python 3 (RHEL 8) | |
| 10:23:22 | aspiers | so the cost of cleaning up six stuff is higher than the performance hit | |
| 10:23:29 | stephenfin | I suspect SUSE and Canonical will be doing something similar | |
| 10:23:31 | stephenfin | Yeah, that ^ | |
| 10:23:46 | sean-k-mooney | aspiers: yes but if we ever packport the code we pay that cost for years | |
| 10:23:50 | bauzas | technically we support py2 for a while | |
| 10:23:52 | sean-k-mooney | hell if we dont we still do | |
| 10:24:01 | bauzas | and the "we" is about the upstream code | |
| 10:24:03 | aspiers | if there are exceptions with like 10000 element dicts then fine we can make an exception for those | |
| 10:24:08 | sean-k-mooney | we have people only finally moving to queens | |
| 10:24:14 | stephenfin | bauzas: Correct. But no distro is packaging it that I'm aware of | |
| 10:24:22 | sean-k-mooney | we will have people on py27 downstream until 2023 | |
| 10:24:33 | stephenfin | sean-k-mooney: There's no performance impact either, right? It's a memory impact | |
| 10:24:39 | stephenfin | Not for new code | |
| 10:24:40 | bauzas | stephenfin: sure but then that's not a good reason for not supporting py27 :) | |
| 10:24:55 | sean-k-mooney | stephenfin: no its slower to use .items or .values on py27 too | |
| 10:24:58 | bauzas | anyway | |
| 10:25:01 | aspiers | this is a classic bikeshed discussion X-D | |
| 10:25:05 | stephenfin | no, but it is a good reason to make an already mostly unnecessary thing even more unnecessary | |
| 10:25:08 | bauzas | aspiers: this | |
| 10:25:11 | stephenfin | I want blue. | |
| 10:25:13 | sean-k-mooney | we are making a copy of all the values and construction an new list | |
| 10:25:21 | bauzas | stephenfin: i want snow | |
| 10:25:25 | aspiers | stephenfin: you can have any blue, as long as it's black | |
| 10:25:30 | sean-k-mooney | bauzas: but anyway do like the new version in general | |
| 10:25:35 | stephenfin | bauzas: You can't handle the snow. | |
| 10:25:44 | sean-k-mooney | bauzas: its closer to what i had originally planned for the funciton | |
| 10:25:44 | bauzas | testtools.matchers._impl.MismatchError: [1, 2, 3, 4] != [1, 2, 4, 3] | |
| 10:25:48 | bauzas | this ^ | |
| 10:25:51 | aspiers | right | |
| 10:25:53 | sean-k-mooney | ya i saw that | |
| 10:25:57 | sean-k-mooney | i was going to fix it | |
| 10:25:58 | cdent | These conversations make me laugh. We're talking about trying to maintain py2 code when real people won't see it for many months, they are all still long time behind now. | |
| 10:26:02 | sean-k-mooney | im relying on dict order | |
| 10:26:09 | sean-k-mooney | wich is only a py36+ thing | |
| 10:26:12 | sean-k-mooney | ill fix that | |
| 10:26:31 | aspiers | sean-k-mooney: you want me to submit my typo/pep8 fixes? | |
| 10:26:39 | stephenfin | bauzas: Oh, here's another one you'll probably want to look at https://review.opendev.org/#/c/674894/ | |
| 10:26:48 | sean-k-mooney | aspiers: you can if you like. | |
| 10:26:48 | cdent | We should be on at least py3.7 everywhere, because by the time the majority of people are using the code... | |
| 10:26:49 | stephenfin | I think you know that code from the vGPU work | |
| 10:27:05 | stephenfin | cdent: only ~ two months to go | |
| 10:27:11 | stephenfin | tick-tock tick-tock | |
| 10:27:18 | cdent | :fist bump: | |
| 10:27:24 | cdent | or :something: | |
| 10:27:58 | bauzas | stephenfin: looking | |
| 10:27:59 | stephenfin | and when we do, I'm doing this _everywhere_ https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:bp/integrate-mypy-type-checking | |
| 10:28:04 | stephenfin | starting with oslo | |
| 10:28:09 | sean-k-mooney | bauzas: i can move the _project_traits function out of the class too. | |
| 10:28:27 | bauzas | sean-k-mooney: honestly, that really depends on what you want | |
| 10:28:37 | bauzas | sean-k-mooney: I'm not particularly opinionated | |
| 10:28:41 | sean-k-mooney | i want it to be a staticmethod | |
| 10:28:47 | bauzas | sean-k-mooney: but I feel we should just make it simplier | |
| 10:29:32 | bauzas | sean-k-mooney: what's fun is that you explain that this method is absolutely unrelated to the driver, but you call it by its libvirt class name :) | |
| 10:29:34 | sean-k-mooney | if we can make it simpler without reducing functionality the sure. have you looked at the folow up patches | |
| 10:29:54 | sean-k-mooney | bauzas: no i want to call it with self | |
| 10:30:22 | bauzas | sean-k-mooney: no I haven't looked at othre patches yet | |
| 10:30:39 | openstackgerrit | Adam Spiers proposed openstack/nova master: Libvirt: report storage bus traits https://review.opendev.org/666914 | |
| 10:30:42 | aspiers | sean-k-mooney: ^^^ | |
| 10:30:43 | sean-k-mooney | @staticmothod is intened to allow you to associate free functions with a class so that they are callable like methods | |
| 10:31:17 | aspiers | sean-k-mooney: also, might be just me, but I would personally prefer test_flatten_iterable to be split into multiple smaller test cases | |
| 10:31:34 | sean-k-mooney | am i could to that | |
| 10:31:36 | aspiers | sean-k-mooney: currently when an assertion fails, you can't immediately see which one failed | |
| 10:31:55 | sean-k-mooney | i built it iteritivly so it was simpler to built up with more test cases | |
| 10:31:59 | aspiers | also giving each a separate method name would add some clarity to exactly what is being tested | |
| 10:32:05 | aspiers | sure | |
| 10:32:22 | sean-k-mooney | i kind of grouped them intentionally expceting that comment :) | |
| 10:32:30 | sean-k-mooney | so ill split it in the next version | |
| 10:32:39 | bauzas | sean-k-mooney: have you clicked on my link ? :) | |
| 10:32:53 | sean-k-mooney | yes | |
| 10:33:34 | sean-k-mooney | that is not my usecase and it is not a good argument | |
| 10:33:49 | sean-k-mooney | i want to be able to call it via self not via the class name | |
| 10:34:05 | sean-k-mooney | i can call it via the class name if im in a non instance/class method | |
| 10:34:20 | sean-k-mooney | but i an instnace method i wanted to call it via self | |
| 10:35:38 | aspiers | sean-k-mooney: cool thanks | |