| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-02 | |||
| 16:57:46 | dansmith | ideally we shouldn't | |
| 16:58:00 | bauzas | because the RPC signature can change ? | |
| 16:58:13 | bauzas | I think I get you | |
| 16:58:26 | sean-k-mooney | dansmith: true although i dont think we relay on that distiction anywhere today but its a valid reason. i take a different appoch and say well the data stucture should be a list so pass an empty one in preference to None | |
| 16:58:28 | bauzas | either way, let's stick with it then | |
| 16:58:41 | bauzas | gibi: I'll reply to your comments | |
| 16:58:50 | gibi | cool | |
| 16:58:53 | gibi | I will check back tomorro | |
| 16:58:54 | gibi | w | |
| 16:59:07 | dansmith | sean-k-mooney: yeah, it's explicit vs. implicit | |
| 17:08:28 | gibi | artom, stephenfin, kashyap: Do we have an agreement on the extra config option vs. +/- prefix in https://review.opendev.org/c/openstack/nova/+/774240/ ? | |
| 17:13:04 | stephenfin | I would still rather separate config opts since that seems clearer and less "unique" to me. I would appreciate other perspectives, though it's definitely into bikeshedding territory | |
| 17:13:57 | gibi | I can work with both but I do understand the uniqueness of the prefix based approach | |
| 17:14:21 | bauzas | stephenfin: dansmith: worth removing this argument now I'm bumping the RPC API ? https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L5311 | |
| 17:14:34 | bauzas | I mean, in https://review.opendev.org/c/openstack/nova/+/761452 | |
| 17:14:34 | gibi | I cannot recall if we discussed this during the meeting when we approved the bp | |
| 17:14:47 | artom | stephenfin, gibi, I don't really care, tbh - stephenfin's idea feels more rigorously correct, but the current +/- approach does have the "way less work" argument on its side | |
| 17:15:11 | artom | And it's not like +/- is super complex to understand. | |
| 17:15:32 | bauzas | stephenfin: dansmith: but I'm fine with punting this for a later minor bump, like 6.1 | |
| 17:15:45 | dansmith | bauzas: you can't drop a param in 6.1 :) | |
| 17:15:51 | artom | I'd be more inclined to go stephenfin's route if the alternative was edging into DSL territory, which it definitely isn't | |
| 17:15:58 | bauzas | oh shit | |
| 17:16:00 | bauzas | you're right | |
| 17:16:14 | bauzas | dansmith: worth the effort then ? | |
| 17:16:26 | stephenfin | No, it's not. It just feels a bit weird. I think I'd even be happier if we had a path to not supporting unprefixed opts | |
| 17:16:31 | dansmith | bauzas: yep, else you wait until 7.0 and those cleanups are the whole point of this | |
| 17:16:57 | bauzas | dansmith: my only concern is that we're passing this argument everytime now | |
| 17:17:02 | stephenfin | gibi: If you don't particularly care, I can drop my complaint. I don't want to have to rework the patch so something is better than nothing | |
| 17:17:05 | bauzas | it's just the manager which doesn't use it | |
| 17:17:06 | gibi | kashyap, stephenfin, artom: so the prefix based solution was raised during the meeting http://eavesdrop.openstack.org/meetings/nova/2021/nova.2021-02-11-16.00.log.html#l-148 | |
| 17:17:25 | gibi | and there was no objection then | |
| 17:17:26 | dansmith | bauzas: no we're not, because 6.0 hasn't landed yet and YOU are defining its behavior :) | |
| 17:17:42 | bauzas | but I'm lazy | |
| 17:17:48 | bauzas | :p | |
| 17:17:49 | dansmith | bauzas: well, that may be :) | |
| 17:18:11 | stephenfin | gibi: yeah, my lack of objections was to the general idea of disabling features rather than how we'd do it (I'd already zoned out by then, as kashyap thought we might have :)) | |
| 17:18:21 | artom | gibi, ah, last week when I was on PTO? :) | |
| 17:18:31 | bauzas | dansmith: I mean, I would have appreciated if someone (like stephenfin :p ) would have stopped to provide this arg by the client | |
| 17:18:36 | bauzas | client isez | |
| 17:18:38 | bauzas | side | |
| 17:18:49 | bauzas | now, we're passing it over the wire | |
| 17:18:54 | bauzas | so we could pretend we never did | |
| 17:18:55 | dansmith | bauzas: in 5.x | |
| 17:19:01 | bauzas | yup | |
| 17:19:06 | dansmith | but not in 6.x | |
| 17:19:11 | gibi | artom: you were present on that meeting :) | |
| 17:19:20 | bauzas | dansmith: right, making it optional | |
| 17:19:31 | bauzas | on the client | |
| 17:19:33 | artom | gibi, oh, hah :P | |
| 17:19:39 | gibi | but good try :P | |
| 17:19:41 | artom | Clearly shows how much I care | |
| 17:19:45 | bauzas | dansmith: but of course, still mandatory on the server until 6.0 | |
| 17:19:56 | artom | Genuinely, we could go either way and I'll sleep perfectly well that night | |
| 17:20:10 | gibi | OK, let's go with the proposed implementation. as I don't see it as a dealbreaker | |
| 17:20:26 | gibi | I'm upgrading my vote on the patch | |
| 17:21:12 | dansmith | gibi: it shouldn't even be optional on the client for 6.0 right? | |
| 17:21:21 | stephenfin | cool, I'll take a look tomorrow | |
| 17:21:26 | gibi | stephenfin: thanks | |
| 17:21:27 | stephenfin | still working through the VNC fun | |
| 17:21:38 | stephenfin | it's a pain in the a*** :) | |
| 17:22:47 | gibi | dansmith: logically accel_uuids are a part of the client code, we do gather them from the flavor and from cyborg. So I think it should be mandatory on the client side as well | |
| 17:22:48 | dansmith | oops s/gibi/bauzas/ above | |
| 17:23:04 | dansmith | gibi: that comment was about request_spec, not accel_uuids | |
| 17:23:08 | gibi | aah | |
| 17:23:11 | gibi | then ignore me | |
| 17:23:16 | dansmith | gibi: I confused you by replying to you instead of bauzas | |
| 17:23:17 | gibi | sorry | |
| 17:23:25 | bauzas | dansmith: right | |
| 17:23:44 | bauzas | OK, looks like I have work to do | |
| 17:28:34 | bauzas | honestly, since this request_spec argument isn't deprecated yet in some 5.x API, removing it now would make 6.0 semantically different from 5.13 | |
| 17:28:55 | bauzas | so I won't do it, unless someone steps up and write a 5.x version for making it deprecated | |
| 17:28:59 | bauzas | stephenfin: ^ | |
| 17:29:23 | bauzas | we lost some opportunity here | |
| 17:30:07 | stephenfin | I haven't been following, I'm afraid :( We're passing an unnecessary argument through to some RPC API? | |
| 17:34:46 | sean-k-mooney | bauzas: how would we get the request_spec if we dont pass it? via the instance? | |
| 17:35:02 | sean-k-mooney | i dont think we want to lazy load that if its currently used | |
| 17:35:19 | bauzas | sean-k-mooney: stephenfin's point is that we don't use this parameter even if we pass it thru the wire | |
| 17:35:37 | sean-k-mooney | for which api call | |
| 17:35:47 | bauzas | and we have a few other methods that have some parameters passed thru the wire that we don't use | |
| 17:36:05 | sean-k-mooney | right so those whould have to be removed in 6.0 | |
| 17:36:06 | bauzas | you know what ? I'm changing the docstring to be 7.0 | |
| 17:36:29 | bauzas | and we know we have to write some 6.x API version that would deprecate those parameters | |
| 17:37:02 | sean-k-mooney | do we have to do deprecations like that for internal rpc apis? | |
| 17:37:07 | bauzas | honestly, now I wrote a 6.0 bump, I feel brave enough for writing a 7.0 one | |
| 17:37:38 | bauzas | but this will have to be at least for X, ideally Y | |
| 17:37:41 | sean-k-mooney | bauzas: well we likely wont do a 7.0 bump for another couple of releases | |
| 17:37:49 | bauzas | yes | |
| 17:37:50 | bauzas | I know | |
| 17:38:01 | sean-k-mooney | proably not before Z | |
| 17:38:22 | sean-k-mooney | i mean there is not strict rule but we have mostly wated 4-6 release between major bumps | |
| 17:38:23 | bauzas | sure, but at least we should stop passing those over the wire if we don't need them | |
| 17:38:29 | bauzas | no | |
| 17:38:42 | bauzas | sean-k-mooney: we released major RPC versions more often in the past | |
| 17:39:13 | bauzas | Kilo, Mitaka, Queens | |
| 17:39:21 | bauzas | and now Wallaby | |
| 17:39:53 | sean-k-mooney | right which is why i said there was no rule that we cant do it more often | |
| 17:40:09 | sean-k-mooney | but we avoid it unless it buys use a lot | |
| 17:40:16 | bauzas | sure, but my concern is to make sure that if we don't need them, we should stop passing them by the client | |
| 17:40:32 | bauzas | hence a 6.x release for stopping this | |
| 17:40:42 | bauzas | and later, a 7.0 for removing | |