| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-12-11 | |||
| 12:07:49 | gibi | then we need a separate patch to re-sync the two | |
| 12:08:10 | sean-k-mooney | yes, i commented on the bug that i was not sure why it needed to be a seperate object vs the nova.object.ImageMetaProps | |
| 12:08:18 | sean-k-mooney | *patch | |
| 12:08:44 | sean-k-mooney | was there a reson you made the payload object sperate? | |
| 12:09:41 | gibi | in general we wanted to have separate objects for notification payload so that we are not enforced to push every internal data out in the notification. But for this particular case if we decide that we always want to push every image meta in the notification then sure in this case we don't need a separate payload class | |
| 12:11:41 | sean-k-mooney | well i guess that is the issue right. having a seperate class made the notification update optional since it would not fail tests if you forgot. | |
| 12:12:24 | sean-k-mooney | ok so ill level the notification updates in the patch for now assuming efried is ok with that and ill file a bug for the fact tehy are not in sync. | |
| 12:12:42 | gibi | sean-k-mooney: ack, it works for me | |
| 12:12:51 | gibi | and thanks for taking care of | |
| 12:13:13 | sean-k-mooney | then we can decide if we want to fix that by extending the new object and added the test for the fields or if we want to just use one object in this case sound good? | |
| 13:53:40 | openstackgerrit | Guo Jingyu proposed openstack/nova master: Make scheduling more debuggable https://review.opendev.org/698421 | |
| 14:00:48 | gibi | sean-k-mooney: sorry, I was pulled. Yeah, it sounds like a plan | |
| 14:01:01 | sean-k-mooney | hehe no worries | |
| 14:16:13 | efried | gibi, sean-k-mooney: ack, thanks for the followup. Sounds like we'll want a big patch to sync the two objects. | |
| 14:16:37 | efried | and maybe some kind of clever test that enforces their parity. | |
| 14:16:50 | efried | for the future | |
| 14:17:12 | gibi | efried: hi! I can get behind this plan | |
| 14:17:34 | efried | This ship has probably sailed, but is there no way to use the same object for both purposes? | |
| 14:18:03 | sean-k-mooney | well the clever test is just loop over the fields dict in the nova.object.image_meta.ImageProp object and assert they are in the notificaiton object dict | |
| 14:18:54 | gibi | efried: if we hack the versioning then I guess we can. But I don't know if we really want to heck the versioning | |
| 14:19:06 | gibi | sean-k-mooney: yeah that could work | |
| 14:19:41 | sean-k-mooney | ill write a patch to do that when i finish updating the functional tests | |
| 14:20:21 | efried | sean-k-mooney: and vice versa | |
| 14:20:26 | efried | thanks sean-k-mooney | |
| 14:21:17 | sean-k-mooney | well actully they are dicts so i could jsut assert the keys are equal | |
| 14:21:41 | sean-k-mooney | if we want the typs to match i could jsut assert the dicts are equal to check the keys and values | |
| 14:26:11 | efried | I'd be fine just asserting they have the same keys. | |
| 14:26:32 | efried | it's really just a sniff test to make sure devs didn't miss syncing. | |
| 14:27:06 | sean-k-mooney | yep ill include it in the sync patch | |
| 14:27:24 | sean-k-mooney | also ddt makes updating the func test way simpler | |
| 14:27:42 | efried | sweet. | |
| 14:28:19 | efried | gibi: what's your vacation schedule? | |
| 14:29:33 | gibi | efried: last official day in the office is 16th. If I cannot finish with the qos live migration until then then I will add some extra time to that before 20th | |
| 14:29:58 | gibi | then back in the 6th of Jan | |
| 14:30:35 | efried | Okay. I'll try to get some reviews in on that. I'm also going to ask for another look at the vTPM spec soon if that's okay. I need to do another update, so maybe tomorrow? | |
| 14:31:21 | mriedem | stephenfin: i've got a few questions in this one https://review.opendev.org/#/c/696509/ | |
| 14:32:15 | gibi | efried: sure, lets do it | |
| 14:32:20 | efried | I'm going to make it simpler. Basically, the bits that are really awkward or hard to explain to users, I'm just going to say "behaves like baremetal". | |
| 14:32:29 | gibi | :) | |
| 14:32:33 | mriedem | stephenfin: also, not sure how others feel about this, but a massive rename like this screws up git history https://review.opendev.org/#/c/696745/7 | |
| 14:32:34 | mriedem | and backports | |
| 14:32:40 | mriedem | so i'd rather not do that personally | |
| 14:32:49 | mriedem | dansmith: how are your feels on this ^? | |
| 14:33:05 | efried | I'm more offended by the misused apostrophe in the commit message. | |
| 14:33:39 | stephenfin | mriedem: It really should. git's automerge tooling should detect renames for us | |
| 14:33:42 | mriedem | i guess https://review.opendev.org/#/c/696745/7/nova/network/api.py isn't as terrible as i would expect | |
| 14:33:45 | stephenfin | *it really shouldn't | |
| 14:34:35 | mriedem | maybe i still have ptsd from when ed renamed all of the legacy v2 contrib api modules | |
| 14:34:36 | dansmith | mriedem: yeah, that seems unnecessary to me | |
| 14:34:51 | stephenfin | There will be merge conflicts but that's due to stuff having been moved around. Straight up file renames aren't an issue | |
| 14:35:08 | dansmith | if others really care about it, then whatever, but it seems more trouble than it's worth to me | |
| 14:35:46 | mriedem | backports related to anything touching networking stuff is going to be a nightmare for awhile anyway, but that's expected | |
| 14:36:01 | efried | so might as well shoot the whole hog? | |
| 14:36:10 | stephenfin | dansmith: Well, I really care about it :) | |
| 14:36:24 | dansmith | this definitely makes backports harder, | |
| 14:36:36 | dansmith | I'm not sure why they're hard without it | |
| 14:36:52 | efried | (hm, I've been misusing that idiom for years. It's "go the whole hog".) | |
| 14:37:33 | efried | dansmith: because of all the *other* nova-net removal churn | |
| 14:37:38 | stephenfin | mocks, for one | |
| 14:38:00 | stephenfin | we no longer need to care about mocking stuff that nova-net was doing. If you backport a test, that goes back to not being true | |
| 14:38:05 | dansmith | efried: okay but that's hard like code conflict, not hard like "this file doesn't exist in the old branch" hard | |
| 14:38:41 | efried | I get it. | |
| 14:39:27 | openstackgerrit | sean mooney proposed openstack/nova master: support pci numa affinity policies in flavor and image https://review.opendev.org/674072 | |
| 14:39:40 | mriedem | anywho, i didn't -1 it and it's later in the series, just mentioning it | |
| 14:39:54 | mriedem | someone has to take on the current big ass bottom change that i +2ed already | |
| 14:39:56 | efried | stephenfin: Given that we have nova/image/glance.py, I don't see a problem with having nova/networking/neutron.py | |
| 14:40:02 | sean-k-mooney | efried: i have not added the spy function to check the pci assignment but i think i have made all the other chagnes you suggested | |
| 14:40:08 | stephenfin | dansmith: but it won't - git is smart | |
| 14:40:12 | mriedem | efried: that's a good point | |
| 14:40:16 | mriedem | we also have a cinder.py | |
| 14:40:24 | efried | nova/volume/cinder.py yeah. | |
| 14:40:45 | stephenfin | dansmith: Go make a trivial modification to nova/tests/unit/test_nova_manage.py, commit on master, then backport to stable/stein | |
| 14:40:47 | efried | we should make sure Sundar names it nova/accelerator/cyborg.py and we're golden. | |
| 14:41:00 | stephenfin | it'll happen cleanly, even though I renamed that file in commit nova/tests/unit/test_nova_manage.py | |
| 14:41:08 | stephenfin | whoops, commit 463017b51b8cde48582b2f55ad7a1f2321d03d02 | |
| 14:41:26 | mriedem | actually i did just -1 | |
| 14:42:23 | mriedem | i think having the module having neutron in the name would be less confusing to someone coming to hack on nova and knowing that there was at least once a nova-network thing, and finding nova/network/api.py and wondering if it's nova-net or neutron | |
| 14:42:34 | sean-k-mooney | efried: that would imply that that code is virt driver indepentet. if they cyborg code in that folder was then sure | |
| 14:42:36 | dansmith | stephenfin: aight, well, when we moved tests/ into tests/unit things were not "smart", but whatevs | |
| 14:42:38 | mriedem | since we have explicit glance.py and cinder.py modules those are pretty clear what they are for | |
| 14:42:52 | mriedem | dansmith: oh yeah the tests -> tests/unit was another big one | |
| 14:43:19 | efried | mriedem: oh, I hope we're getting rid of the API shim | |
| 14:43:27 | efried | stephenfin: ? ^ | |
| 14:43:34 | mriedem | efried: i'm fine with removing base_api.py and api.py (nova-net) | |
| 14:43:42 | mriedem | but i don't think we need to rename neutronv2.api to api.py | |
| 14:43:55 | efried | yeah, I'm fine leaving it named neutron | |
| 14:44:04 | sean-k-mooney | well on that how would people feel about eventurally moving the neutron code to os-vif? | |
| 14:44:34 | mriedem | why? | |
| 14:44:36 | efried | that sounds like a conversation for a far future release | |
| 14:44:57 | sean-k-mooney | well partly to not need to have any netowrking code in nova | |
| 14:44:59 | mriedem | "hey let's move some already really complicated and not very well understood code out to a library with a different core team" | |
| 14:45:10 | canori01 | Is it safe to rebuild the placement database? I have an issue where all my hypervisors are running into conflicts (conflicting resource provider name) | |
| 14:45:16 | sean-k-mooney | well all nova cores are os-vif cores | |
| 14:45:19 | canori01 | Can I just empty the db and bounce nova-compute? | |
| 14:45:27 | sean-k-mooney | but anyway its just an idea | |
| 14:45:33 | mriedem | sean-k-mooney: not worth the effort | |
| 14:45:38 | mriedem | pick your battles | |
| 14:45:56 | sean-k-mooney | its currently plugable so i was thinking of porting it then we could swap after | |
| 14:45:58 | sean-k-mooney | but ok | |
| 14:46:01 | mriedem | canori01: the inventory will rebuild itself automatically, the consumers/allocations will not | |