| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-02 | |||
| 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 | |
| 17:40:46 | sean-k-mooney | with that said not that we enforce the min compute version on startup there is less utility in keeping a major version for a long time | |
| 17:42:44 | bauzas | sean-k-mooney: again, stop thinking about the next major RPC release | |
| 17:43:11 | bauzas | sean-k-mooney: just a single minor version for stopping to emit this param and I'll be happy | |
| 17:43:34 | sean-k-mooney | sure but to do that you need to make them optional parmaters right | |
| 17:43:41 | sean-k-mooney | and then just stop setting them | |
| 17:44:26 | sean-k-mooney | so you need to prepare for it in the 6.0 version | |
| 17:45:01 | sean-k-mooney | unless you were planning to keep them mandatoy and have the clinet hardcode None or something? | |
| 17:47:25 | sean-k-mooney | i dont think i have review a patch were we actully droped parmaters. i know matt did it before but im not sure what the process is | |
| 17:54:19 | sean-k-mooney | https://github.com/openstack/nova/commit/a761e57368280b6d3e931831ecd393fd5787b3ef dropped the compat code for 4.x but the parmaters i think were doped in teh 5.0 bump that added the 4.x proxy | |
| 17:54:21 | sean-k-mooney | https://github.com/openstack/nova/commit/eae37a27caa5ca8b0ca50187928bde81f28a24e1#diff-91f79786d7e3744c39926c88bbafe3b727630fa4eb48e845686d7f12f876d067L531 | |
| 18:05:02 | openstackgerrit | Merged openstack/nova master: Docs: Correct ``Password injection using the dashboard`` Explanation https://review.opendev.org/c/openstack/nova/+/775084 | |
| 18:09:53 | dansmith | sean-k-mooney: we only ever drop params in a major bump, and all such bumps have dropped parameters, AFAIR | |
| 18:10:31 | dansmith | sean-k-mooney: you need to continue to honor them for N-1.x compatibility in that shim, but the N.0 client and server can assume everything is always passed and expected at the new signatures you want going forward | |
| 18:11:05 | sean-k-mooney | dansmith: yes that is what i understood too | |
| 18:11:09 | dansmith | ack | |
| 18:15:44 | dansmith | bauzas: to be clear, we do not need to do any sort of "deprecate in 6.x and remove in 7.0" dance for _anything_. We're the only consumer of this API, so we only need to make sure we honor our own rules.. | |
| 18:16:08 | dansmith | The entire 5.x lineage is adding things to the API as optional, to be made mandatory in 6.0, and ignoring things we plan to drop in 6.0 | |
| 18:16:52 | dansmith | so all you need to do is support 5.max and 6.0 in both the client and server, and the only thing that you can't do is break 5.max.. 6.0 could be anything else you want if you're willing to make the changes | |
| 18:19:06 | bauzas | dansmith: ok, I'll see what I can do | |
| 18:19:20 | bauzas | the 6.0 bump already has a long list of changed things | |
| 18:19:28 | bauzas | and I'm just about cleaning it more | |
| 18:19:42 | bauzas | I'm the janitor and it's a mess | |