| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-02 | |||
| 16:18:51 | gmann | if do so then we endup checking two (PUT and SWAP) policy for swap | |
| 16:19:27 | dansmith | I strongly feel that going forward this should behave like a regular PUT, where you can PUT with no changes all the time, | |
| 16:19:38 | dansmith | and only check the swap policy if you're actually changing the volume | |
| 16:19:45 | dansmith | I don't see how anything else makes sense | |
| 16:21:44 | gmann | dansmith: yeah +1 on 'only check the swap policy if you're actually changing the volume'. but this became hard to implement with no value check of dot. | |
| 16:22:00 | gmann | i think if you rebase the policy patch then it will be clear. | |
| 16:22:28 | dansmith | okay I don't see what the value of dot has to do with which policy gets checked | |
| 16:22:38 | gmann | policy stuff is a challenge here | |
| 16:22:50 | bauzas | gibi: if you don' mind, I would appreciate https://blueprints.launchpad.net/nova/+spec/vgpu-multiple-types to be targeted for ussuri-3 | |
| 16:23:08 | dansmith | gmann: do you not agree that if you have swap permissions, you should also be able to change dot? | |
| 16:23:09 | bauzas | the patches in question are small | |
| 16:23:36 | gmann | dansmith: that is fine but vice versa should not | |
| 16:23:42 | gmann | dansmith: here - https://review.opendev.org/#/c/711194/11..13/nova/api/openstack/compute/volumes.py@468 | |
| 16:23:46 | gibi | bauzas: noted, I will update lauchpad after the meeting | |
| 16:23:56 | gmann | otherwise we end up doing PUT policy check always | |
| 16:24:21 | bauzas | gibi: thanks | |
| 16:24:22 | dansmith | gmann: right but as stephen (and now I) say there, that's the wrong place to check policy | |
| 16:24:58 | dansmith | gmann: we should check at the beginning of update(), either the PUT or PUT-with-swap policy based on whether or not volumeId is changed.. it should be very simple if..else | |
| 16:25:41 | dansmith | gmann: anyway, once the base patch looks like it's +2 I will rebase the policy patch (or help brinzhang_ ) and we can discuss/argue the policy bit on that | |
| 16:27:03 | gmann | dansmith: ok. after rebase it will be more clear. But i think i got your point of swap policy supersede the PUT policy | |
| 16:27:18 | openstackgerrit | Dan Smith proposed openstack/nova master: Allow PUT volume attachments API to modify delete_on_termination https://review.opendev.org/693828 | |
| 16:27:23 | dansmith | gmann: ack | |
| 16:27:30 | dansmith | gmann: ^ that removes one more thing alex asked about that I forgot | |
| 16:27:41 | gmann | ok | |
| 16:28:13 | dansmith | gmann: sorry if it got lost in the policy discussion, but... is there still a need to change the schema for null-ness? | |
| 16:29:41 | gmann | with 'x in attr' check, it is not needed | |
| 16:29:54 | dansmith | gmann: okay cool | |
| 16:31:09 | stephenfin | dansmith: off the top of your head, obj_from_primitive is called whenever we pull an o.vo from the DB, yeah? | |
| 16:31:31 | dansmith | stephenfin: for serialized objects | |
| 16:36:21 | bauzas | artom: FWIW, I left the bare minimum in the etherpad for your OSC concerns, the floor is yours :p | |
| 16:36:28 | bauzas | artom: https://etherpad.openstack.org/p/nova-victoria-ptg | |
| 16:36:31 | artom | bauzas, OK | |
| 16:36:44 | artom | bauzas, I'll proll'y just be lazy and harass mordred | |
| 16:37:01 | bauzas | I think the problem isn't the design as I said | |
| 16:37:06 | artom | About where any new microversion-enabling client code should go | |
| 16:37:10 | bauzas | we discussed it so many times | |
| 16:37:17 | bauzas | I think we just need hands | |
| 16:37:32 | artom | bauzas, well that's the point, right? We already say "if you're adding a new microversion, you must provide the novaclient patch" | |
| 16:37:41 | artom | Not we'll be saying "and you also need to provide the osdk patch" | |
| 16:37:45 | artom | If osdk is indeed the correct place | |
| 16:37:45 | bauzas | artom: it's more complicated than that | |
| 16:37:56 | bauzas | artom: if you're about to propose, say 2.81 | |
| 16:38:10 | bauzas | artom: then you also need to make sure that <2.81 works too | |
| 16:38:29 | bauzas | and then you signed-off for doing all the stuff, yay | |
| 16:38:43 | artom | bauzas, oh, because of the gap | |
| 16:38:45 | artom | *sigh* | |
| 16:38:52 | artom | Yeah ok, I get it | |
| 16:38:54 | mordred | well - once we reach steady-state making sure <2.81 works too should be pretty easy | |
| 16:39:30 | artom | mordred, what does steady-state mean in this context? | |
| 16:39:31 | mordred | that said - if we can just start getting folks to submit an osdk patch wehn new microversions are made so we can start tracking new things, we can use those as opportunities to fill in old stuff - and I'm happy to help do that | |
| 16:39:48 | artom | mordred, I think I'd like that - we have to start somewhere | |
| 16:39:50 | mordred | artom: I mean havig filled in the support for the existing old microversions | |
| 16:40:04 | artom | mordred, ah, that was the thing that mriedem started | |
| 16:40:09 | mordred | most of the time when someone says "I want to support microversion vX" - it turns out ot be quite easy | |
| 16:40:10 | artom | Well, started driving | |
| 16:40:12 | mordred | yeah | |
| 16:40:24 | mordred | but - there's a bunch we need to walk through and make sure we understand :) | |
| 16:40:34 | artom | mordred, in terms of how the Nova API works? | |
| 16:40:49 | mordred | yeah- many of them wind up being no-ops from sdk point of view | |
| 16:41:00 | mordred | but it's good to go look at the MV and say "yup, nothing to do | |
| 16:41:20 | artom | mordred, ah, for example for things like new fields in responses? sdk just shows them with no added logic | |
| 16:41:25 | artom | (Right?) | |
| 16:41:26 | gmann | yeah, like adding the field in response is same list | |
| 16:42:04 | mordred | yeah, well, we need to add new fields when they're added | |
| 16:42:27 | mordred | but - we already are up to date with most fields as of like the mid-2.4x or 2.5x - just not labeled that way | |
| 16:42:37 | mordred | so for many of them we're already caught up :) | |
| 16:42:48 | mordred | and for new ones adding support is _usually_ very trivial - one or two lines | |
| 16:43:00 | mordred | the behavior breaks - like changing live migration - those take some real thought | |
| 16:43:08 | mordred | but - those require real thought to do the feature too | |
| 16:44:20 | artom | Yeah, the live migration thing was ugly | |
| 16:44:32 | artom | But let's not revisit that :) | |
| 16:44:33 | gmann | is it not we need to bump the python-novaclint version on OSC because we maintain the MAX_VERSION there and increase when no change needed | |
| 16:44:38 | mordred | yeah. but - that's one of the ones where I'm quite happy with our support | |
| 16:45:36 | mriedem | in my experience adding feature parity for compute API microversions to OSC, for example, is not always so trivial, especially when the people reviewing the changes aren't from the nova team or aren't familiar with how the API works | |
| 16:46:12 | gmann | +1 | |
| 16:46:13 | mordred | I agree - although I'd like to make progress on changing that :) | |
| 16:46:14 | artom | mordred, alright, we'll probably talk this over at the PTG again, but the initial "action item" of "require sdk patch, use as opportunity to catch up a bit" makes sense to me | |
| 16:46:17 | mriedem | so random person throws up a change and the OSC team just basically trusts that it's correct from an API POV and then their review is based on UX standards for OSC | |
| 16:46:37 | mriedem | mordred: there was progress while i was pushing it from the nova team, but ... | |
| 16:46:41 | mriedem | someone needs to fill those shoes | |
| 16:46:49 | artom | Yeah, having someone from the project reviewing the change should be a must | |
| 16:46:50 | mordred | mriedem: we just need to clone you | |
| 16:47:14 | artom | Doesn't have to be core | |
| 16:47:21 | gmann | I can volunteer for that | |
| 16:47:26 | artom | Like, I feel I have a good enough handle on it to give +1s | |
| 16:47:29 | artom | (Or -1s) | |
| 16:47:35 | mriedem | no it doesn't have to be a core, but it's good to be familiar with the actual API change and spec and all that | |
| 16:47:42 | artom | ... which I never did, so mea culpa, I guess | |
| 16:47:49 | mriedem | and the people that review API changes and specs are ... usually cores | |
| 16:48:14 | artom | Well, assuming the api-ref is correct and complete, that's good enough doc, no? | |
| 16:48:26 | mriedem | anyway, if the api side core review enforces good docs / api ref then anyone should be able to review the client side changes | |
| 16:48:31 | mriedem | jinx | |
| 16:48:41 | artom | And if you review the sdk change, look at the api-ref, and can't grok if it makes sense or not, it means we've failed at api-ref | |
| 16:48:54 | mriedem | lots of ifs | |
| 16:49:05 | artom | And butts | |
| 16:49:10 | artom | (Sorry) | |
| 16:49:12 | mriedem | candy and nuts | |
| 16:49:17 | gmann | mriedem: how you do, monitoring yourself or osc team add as reviewer etc ? | |
| 16:49:29 | gmann | you did | |
| 16:49:58 | mriedem | i'd have to find my old etherpad that was tracking the compute API microversion gaps in OSC | |