Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-02-23
14:28:56 bauzas a parameter being {} by default ?
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

Earlier   Later