| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-02-23 | |||
| 14:29:04 | bauzas | I hope we don't do it | |
| 14:29:19 | bauzas | or maybe I misunderstood | |
| 14:29:20 | gibi | bauzas: nope, a singalling of a failure here https://github.com/openstack/nova/blob/8ca5d2169fc3ba5cf86f830a93cf92bae925e1f4/nova/network/neutron.py#L650 | |
| 14:29:25 | gibi | sean-k-mooney: good point | |
| 14:29:45 | gibi | sean-k-mooney: but if we only have | |
| 14:29:47 | gibi | a = {} | |
| 14:29:52 | gibi | if a: | |
| 14:30:03 | gibi | then this code does not have a key type defined either | |
| 14:30:14 | sean-k-mooney | thats fine | |
| 14:30:21 | sean-k-mooney | you dont need to define a key type | |
| 14:30:25 | sean-k-mooney | you can just use dict | |
| 14:30:27 | gibi | I can assign a: ty.Dict = {} | |
| 14:30:42 | sean-k-mooney | you can but ty.Dict is an alis for dict | |
| 14:30:50 | gibi | true | |
| 14:30:51 | gibi | thank | |
| 14:30:52 | gibi | s | |
| 14:31:17 | bauzas | I don't understand the concern but meh to mypy | |
| 14:31:22 | sean-k-mooney | dict is treated like ty.Dict[ty.Any,ty.Any] | |
| 14:31:27 | bauzas | defaultdict maybe ? | |
| 14:31:37 | gibi | sean-k-mooney: yep it is | |
| 14:32:22 | gibi | bauzas: if you infer types from the usage of a dict then if the dict is never used (empty) then you cannot infer a key type | |
| 14:32:31 | gibi | bauzas: so it is different that defaultdict | |
| 14:32:52 | bauzas | ah I see | |
| 14:33:12 | sean-k-mooney | bauzas: gibi just wanted to be a bit more explcit about the contained types then dict of anything | |
| 14:33:26 | sean-k-mooney | whcih he can do explitcly with the : syntax | |
| 14:33:26 | bauzas | that's one of the reason why I prefer to signal an nullable variable as None | |
| 14:33:46 | sean-k-mooney | well its not really nullable | |
| 14:33:51 | gibi | bauzas: yes, {} and None has a different meaning | |
| 14:34:00 | gibi | even if we tend to mix them | |
| 14:34:06 | sean-k-mooney | they are different types | |
| 14:34:19 | bauzas | sean-k-mooney: I know, I'm just saying that I prefer to have an object with a None value rather than an id and which is empty | |
| 14:34:47 | sean-k-mooney | there are pros and cons to both | |
| 14:36:05 | sean-k-mooney | using Empty collections is generally perfered using None for literals or classes is also genereally perfered | |
| 14:36:26 | sean-k-mooney | None tened to be used for singal values and empty collection fo correct type for aggreates | |
| 14:49:12 | sean-k-mooney | stephenfin: it will be a while before i push anything but how do you feel about hw:mem_locked=True|False as the name of the new extra spec required for vdpa | |
| 14:50:17 | sean-k-mooney | so vdpa guest woudl need hw:mem_locked=true and hw:mem_page_size=<anything> set | |
| 14:50:42 | sean-k-mooney | mem_page_size to account for no over commit and correct tracking | |
| 14:50:56 | jkulik | hi, we just upgraded to rocky. is the "sync aggregates" command supposed to be usable with ironic nodes? | |
| 14:50:58 | sean-k-mooney | and locked for the the dma limiations | |
| 14:52:49 | jkulik | because it raises an TooManyComputesForHost ... is there an official way to migrate those aggregates? | |
| 15:03:19 | stephenfin | gibi: Not a clue. That seems like an odd request | |
| 15:03:42 | gibi | stephenfin: I stick to dict (aka ty.Dict) | |
| 15:03:49 | gibi | it works | |
| 15:03:52 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Turn on mypy for nova/network/neutron.py https://review.opendev.org/c/openstack/nova/+/777146 | |
| 15:04:06 | gibi | stephenfin, bauzas: turned on mypy for nova/network/neutron.py | |
| 15:04:14 | bauzas | gibi: yeah saw it | |
| 15:04:24 | gibi | I will rebase that on top of the routed net patch | |
| 15:04:24 | bauzas | will look at that and thanks for working on it | |
| 15:05:56 | openstackgerrit | Lucas Alvares Gomes proposed openstack/nova master: [OVN] Explicitly set nova-next job to ML2/OVS https://review.opendev.org/c/openstack/nova/+/776944 | |
| 15:05:57 | openstackgerrit | Lucas Alvares Gomes proposed openstack/nova master: DO NOT REVIEW: Test OVN devstack module https://review.opendev.org/c/openstack/nova/+/776945 | |
| 15:07:10 | sean-k-mooney | i kind of think ^ is inverted | |
| 15:07:39 | sean-k-mooney | we shoudl proably explcity set nova-next to ovn and keep the rest on ml2/ovs for now and then consider swaping to ovn on more jobs later | |
| 15:16:06 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Turn on mypy for nova/network/neutron.py https://review.opendev.org/c/openstack/nova/+/777146 | |
| 15:20:15 | gibi | stephenfin: I've replied in https://review.opendev.org/c/openstack/nova/+/772381/6/nova/virt/libvirt/driver.py#263 | |
| 15:24:51 | stephenfin | ta | |
| 15:35:36 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Move the guest.get_disk test to test_guest https://review.opendev.org/c/openstack/nova/+/777151 | |
| 15:37:58 | lpetrut | Hi, about the Windows RBD patch: would you guys be ok with skipping the os-brick dependency until we sort out the lower constraints issue? https://review.opendev.org/c/openstack/nova/+/763550 | |
| 16:02:59 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Enable mypy on libvirt/guest.py https://review.opendev.org/c/openstack/nova/+/777155 | |
| 16:03:15 | gibi | stephenfin: this is an interesting one ^^ | |
| 16:04:03 | stephenfin | looking | |
| 16:06:15 | gibi | stephenfin: I did that as a followup for the detach bugfix series | |
| 16:06:53 | stephenfin | gibi++ Yup, realize that. Thanks :) | |
| 16:07:01 | stephenfin | So the unusual thing is the assertion, I assume? | |
| 16:08:54 | stephenfin | gibi: Is it just me, or are all those functions in the exception handlers than are accepting a 'guest' parameter broken? | |
| 16:09:11 | stephenfin | If so, I bet I was the one that broke it too. I recall reworking that stuff recently | |
| 16:09:27 | stephenfin | and attempts to bury the evidence ;) | |
| 16:10:27 | gibi | hm, you have a point, mypy only found the last usage of gues but the expcetion handler code also depends on the guest variable | |
| 16:11:33 | gibi | _cleanup_failed_start checks the guest before use it | |
| 16:11:47 | gibi | so the exception handlers are good | |
| 16:11:58 | stephenfin | the guest is useless though | |
| 16:12:08 | stephenfin | there's no way a function that raises an exception can return something | |
| 16:12:21 | stephenfin | so guest will either always be None or it will be set and an exception won't be raiseed | |
| 16:12:23 | stephenfin | *raised | |
| 16:12:46 | stephenfin | I suspect there was another caller after '_create_guest' previously | |
| 16:12:49 | stephenfin | power on or something | |
| 16:14:05 | gibi | hm, yeah, this can be cleaned up to only run the code that is currently in the finally block of _cleanup_failed_start | |
| 16:15:06 | stephenfin | yeah, https://github.com/openstack/nova/blob/stable/train/nova/virt/libvirt/driver.py#L6351-L6352 | |
| 16:15:55 | stephenfin | there was stuff done after '_create_guest' (formerly '_create_domain') previously | |
| 16:16:20 | stephenfin | so I suspect the real solution to that is to stop passing 'guest' to '_cleanup_failed_start' and remove any code that relied on it? | |
| 16:16:38 | stephenfin | which is what you just said, sorry :) | |
| 16:16:46 | gibi | I don't know why we try to stop the guest | |
| 16:16:57 | gibi | nvm I see it | |
| 16:17:09 | stephenfin | why or where? | |
| 16:17:10 | gibi | so in the past there was a case when the guest was fully started | |
| 16:17:16 | gibi | and then we failed | |
| 16:17:19 | gibi | so we need to stop is | |
| 16:17:20 | gibi | it | |
| 16:17:37 | gibi | but now it either starts without exception or fails before it is started | |
| 16:17:37 | stephenfin | if the firewall driver couldn't apply filters, right? | |
| 16:17:42 | gibi | yepp | |
| 16:17:49 | stephenfin | yeah, that's my reading of this too | |
| 16:18:04 | stephenfin | I did the firewall cleanup too but missed that. Apologies | |
| 16:18:53 | gibi | hm, I don't know what happens if create_guest fails at libvirt_secret.undefine() in the finally block, at that point we have a running guest I assume | |
| 16:19:20 | gibi | but we don't get back the guest object from the caller | |
| 16:19:25 | gibi | to stop it | |
| 16:19:59 | stephenfin | yeah, an exception will still be raised | |
| 16:20:44 | stephenfin | the finally block just executes stuff before the finally happens, iirc | |
| 16:20:52 | stephenfin | akin to save_and_reraise | |
| 16:22:58 | gibi | so we have to catch possible exceptions from libvirt_secret.undefine(), LOG them, and still return the geust object | |
| 16:23:07 | gibi | guest | |