| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-08-23 | |||
| 11:19:41 | sean-k-mooney | efried: i assume we are talking about cyborg privsep | |
| 11:19:48 | efried | yes | |
| 11:20:46 | sean-k-mooney | i think i was a little harsh in the review but i also have strong feeling about this topic. | |
| 11:26:28 | efried | Swhy I was hoping you would look at it. | |
| 11:47:05 | sean-k-mooney | efried: well i review in cyborg octionally and is mostly negitve when i do. not alway but my +/- raition there is proably not that positive. so i do try to make sure my review that i do there are at least constructive rather then saying "no this is all terible" and suggest how it can be improved. but if i do my usuall 50 billion comments on each indiviaual issue that can come across badly if in a | |
| 11:47:07 | sean-k-mooney | "drive by" review. | |
| 11:53:30 | openstackgerrit | Eric Fried proposed openstack/nova master: Use SDK for setting instance id https://review.opendev.org/659690 | |
| 11:53:31 | openstackgerrit | Eric Fried proposed openstack/nova master: Use SDK for add/remove instance info from node https://review.opendev.org/659691 | |
| 11:53:31 | openstackgerrit | Eric Fried proposed openstack/nova master: Use SDK for getting network metadata from node https://review.opendev.org/670213 | |
| 11:54:53 | efried | sean-k-mooney: Any update on that libvirt attribute ordering bug? Not sure if I missed something going past... | |
| 11:58:12 | sean-k-mooney | efried: its still on my todo list. i was dealing with a downstream backport the last day or two but i should get to it today or monday. | |
| 11:58:50 | efried | cool, thanks. It's a "requirements freeze" thing fwiw. | |
| 11:59:12 | sean-k-mooney | oh ya when is that next week? | |
| 11:59:31 | sean-k-mooney | i was aware of that but i proably should expidite looking at it | |
| 11:59:56 | sean-k-mooney | non-client lib frezee shoudl m3 -2 weeks | |
| 11:59:57 | efried | oh, I thought it was, but no, it's the week of Sept 9 | |
| 12:00:05 | sean-k-mooney | and oslo shoudl be m3 -3 | |
| 12:00:16 | sean-k-mooney | i guess requirement freeze is m3-1 | |
| 12:01:12 | sean-k-mooney | actully it looks like its m3 | |
| 12:01:35 | sean-k-mooney | so september 12th | |
| 12:01:58 | sean-k-mooney | but ill try and repoduce anyway this/next week and get a patch up so we dont run up against it | |
| 12:03:20 | sean-k-mooney | is this a shorter release by the way. it feels a little compressed. the freeze dates are all 1 week later then i expect in my head | |
| 12:04:29 | sean-k-mooney | maybe im just used to makeing sure everythin is in os-vif on time and i naturally try to have the last week to make sure we dont have any bugs before the freeze | |
| 12:49:53 | dougsz | nova-manage db online_data_migrations <- Do I need to run that per cell, or is that a top level only thing? Ref: https://docs.openstack.org/nova/latest/user/upgrade.html | |
| 12:51:01 | AJaeger | nova stable cores, could I get a second +2 on https://review.opendev.org/677178, https://review.opendev.org/677176 https://review.opendev.org/677177, please ? Those remove experimental openSUSE 42.3 jobs since the distro is end-of-life now. | |
| 12:54:22 | sean-k-mooney | AJaeger: do we have a replacement suse job either based on tumbelweed or a more recent leap release | |
| 12:55:21 | sean-k-mooney | AJaeger: or rather do we have opensuse node sets so a replacment job could be built if needed | |
| 12:57:12 | AJaeger | sean-k-mooney: yes, in master and stein already | |
| 12:57:25 | sean-k-mooney | cool | |
| 12:57:51 | AJaeger | but I didn't want to add new distro support to old stable branches that only have an experimental job | |
| 12:58:08 | sean-k-mooney | ya makes sense | |
| 12:58:27 | AJaeger | sean-k-mooney: there's job tempest-full-py3-opensuse150: | |
| 12:58:39 | AJaeger | (still experimental) in master and stein | |
| 12:59:17 | sean-k-mooney | im not sure experimental jobs add much value on stable as they will be used even less often then master. but its still better then nothing | |
| 13:01:28 | sean-k-mooney | or in other words removing an experimal job form stable that is on an unsupproted os/is broken is pretty safe | |
| 13:02:46 | AJaeger | agreed | |
| 13:57:18 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Do not log token in the novncproxy logs https://review.opendev.org/678234 | |
| 14:05:37 | openstackgerrit | Eric Fried proposed openstack/nova master: Update SDK fixture for openstacksdk 0.35.0 https://review.opendev.org/678237 | |
| 14:11:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Process [compute] in $NOVA_CPU_CONF in nova-next https://review.opendev.org/672800 | |
| 14:11:53 | alex_xu | stephenfin: nice | |
| 14:13:25 | alex_xu | mriedem: I replied your email, let me know what can make you have more confidence on this | |
| 14:15:46 | dansmith | artom: you've got a bunch of legit unit test fails | |
| 14:16:15 | artom | dansmith, yeah, those have been fixed locally | |
| 14:16:16 | dansmith | artom: I'm also concerned about the validity of some of the other tests that aren't failing even though you're using the wrong dst_numa_config in some of the code and/or tests | |
| 14:16:37 | dansmith | artom: okay cool, I just lightly dusted the bottom several patches with more comments | |
| 14:16:41 | artom | dansmith, I think that's because I was asserting obj.dst_numa_config and setting it as well | |
| 14:16:46 | dansmith | up to the one you said was the top of the things you've updated | |
| 14:17:08 | artom | So the code sets dst_numa_config, and the test checks it as well | |
| 14:17:12 | dansmith | artom: okay, I guess just setting a non-field property on the object, okay | |
| 14:17:20 | artom | dansmith, yeah | |
| 14:17:34 | dansmith | well, let's make sure to get all those right :D | |
| 14:17:45 | artom | dansmith, so you're saying I need to squash the RPC changes into the top patch? | |
| 14:17:49 | dansmith | I guess no real testing was done with the updated set since the top weren't updated anyway | |
| 14:17:58 | artom | I split them to keep patch size as small as possible | |
| 14:18:04 | artom | But looks like it'll be unavoidable here :( | |
| 14:18:11 | artom | dansmith, yeah, grep took care of that | |
| 14:18:45 | dansmith | artom: squash the rpc one with the top one because of the version/capability advertisement? | |
| 14:18:52 | artom | dansmith, yeah | |
| 14:19:18 | dansmith | that's probably too obtuse of a solution.. I mean it'll work I guess, but hopefully we can split them a little more than that | |
| 14:19:37 | dansmith | flipping their order is probably better | |
| 14:20:03 | artom | Ohhh | |
| 14:20:11 | dansmith | get the code in place, just don't allow it to be called from RPC until the second one, where you actually bump the versions and add the params | |
| 14:20:12 | dansmith | and remove the extra service version bump of course | |
| 14:20:13 | dansmith | amirite? | |
| 14:20:44 | artom | Makes sense, though I need to think through how that works for params | |
| 14:21:04 | dansmith | it'll require some tweaking of things I'm sure, but that general ordering should work for you I think | |
| 14:21:34 | dansmith | artom: I also think you probably need to maintain a -W on the bottom patch, if you're going to have it in various states of not-really-working (or I can -2 it) | |
| 14:21:57 | artom | It's that sort of expected as the series goes through review? | |
| 14:22:02 | dansmith | artom: since it's going to mostly require a manual verification (which right now would fail hard), we probably want to wait to land much of it until it's actually proven to be working | |
| 14:22:03 | artom | But I can do that, sure | |
| 14:22:12 | artom | Ah, in that sense | |
| 14:22:18 | artom | Yeah, makes sense | |
| 14:22:24 | dansmith | just for someone not really paying attention, | |
| 14:22:36 | dansmith | because your bottom few patches look devilishly small | |
| 14:23:04 | artom | I'm devious like that :D | |
| 14:23:41 | artom | Actually the func test on top is only failing because I explicitly added an assertFalse because I wanted to see the logging | |
| 14:23:41 | dansmith | the claim one wouldn't be terrible to land in isolation, but the next one is an object schema commitment, which is still not ready to land, so... | |
| 14:24:00 | artom | I can remove that so we can get at least some confidence in the gate | |
| 14:24:02 | dansmith | okay I'm not sure how that can be the case with you using the wrong field names in the object, | |
| 14:24:08 | dansmith | unless it's really that fake | |
| 14:24:23 | artom | dansmith, well ok, it *was* failing because of the assertFalse | |
| 14:24:28 | dansmith | oh okay I see | |
| 14:24:32 | artom | *now* it might be broken for real :D | |
| 14:24:37 | dansmith | I hope it is :) | |
| 14:24:44 | dansmith | otherwise it doesn't do much for confidence :) | |
| 14:25:13 | artom | dansmith, yeah: "AttributeError: 'module' object has no attribute 'NUMAHostInfo'" | |
| 14:27:09 | efried | artom: qq: Est-ce que c'est vrai que, si je dis, "on ne permet pas l'un ou l'autre," même si tu connais ce que je veux dire, ce n'est pas strictement correcte? Il faudrait dire, "on ne permet pas ni l'un ni l'autre"? | |
| 14:27:38 | artom | efried, on ne permet ni l'un ni l'autre | |
| 14:27:49 | efried | sorry, yah | |
| 14:28:01 | artom | Ou même "aucun des deux n'est permis" | |
| 14:28:37 | dansmith | artom: uh, that's not the one I was worried about, so maybe there's another problem? | |
| 14:28:48 | artom | dansmith, it probably just hit that one first | |
| 14:28:59 | dansmith | point is, I didn't catch that one | |
| 14:29:45 | artom | dansmith, oh, wait, that's from an unrelated stephenfin commit | |
| 14:29:57 | artom | dansmith, tell you want, before addressing any other feedback, I'll un-WIP the func test | |
| 14:30:04 | artom | *what | |
| 14:30:14 | dansmith | un-WIP and fix? | |
| 14:30:20 | artom | Yeah, fix was implied :) | |
| 14:30:28 | dansmith | juuuuust checking :) | |
| 14:30:43 | artom | Haha, there are limits to my lazyness ;) | |