Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-06-06
12:11:42 stephenfin I was thinking I could move this "delete additional properties" logic into the various affected methods themselves, but that's a big list and I don't know if there's any way to know we're handling a v2 request vs. a v2.1+ request
12:11:44 gibi I would inclined to simply deprecate it as we cannot maintain it any more
12:11:55 stephenfin yeah, that's the other option: delete v2
12:12:04 sean-k-mooney is something like this valid https://paste.opendev.org/show/bkS797jQUZZoSNXnhlvO/
12:12:30 sean-k-mooney and if so is oneOF valid as the value of required
12:12:45 sean-k-mooney also have you asked gemini :)
12:14:23 sean-k-mooney ok it has a idea
12:14:33 sean-k-mooney https://paste.opendev.org/show/bMERk0Vt08kwAdUW6tLi/
12:14:46 sean-k-mooney let me ask it with the full example
12:14:54 gibi hm does the validator in the _soft_validate_additional_properties sees the whole big schema? or has some kind of memory when the validation descends the tree? Could we use that validator object to stop applying the rule in the subtree of all oneOfs dinamically?
12:15:39 stephenfin sean-k-mooney: ah, no that won't work (and I actually recall trying it before): you can't combine additionalProperties with oneOf https://github.com/python-jsonschema/jsonschema/issues/193
12:16:11 stephenfin https://ajv.js.org/faq.html#additional-properties-inside-compound-keywords-anyof-oneof-etc
12:17:15 stephenfin gibi: it does not, at least not with oneOf. It gets handed the subschemas individually
12:18:54 stephenfin and the issue is that e.g. if I'm given a body like {'server': {...}, 'invalidkey': 'stuff'} then I probably want to strip out 'invalidkey' (a made up example but you get my point)
12:20:34 sean-k-mooney https://paste.opendev.org/show/828072/
12:20:50 sean-k-mooney stephenfin: ^ still readign that but that what gemini thinks about the problem
12:22:37 stephenfin Not a fan of approach 1. I really don't want to encode logic in our schemas. They're already complex enough
12:23:20 sean-k-mooney for this specific case i tought that was possibel ok if it was only this one case we neede to handel for 2.0 validation
12:23:21 stephenfin Hmm, actually: maybe I can just disable response schema validation entirely for v2.0...
12:23:36 sean-k-mooney that was gong to be my other question
12:23:44 sean-k-mooney v2.0 is technially unversioned
12:23:51 stephenfin It doesn't fix the problem but it does side-step it
12:23:57 sean-k-mooney so it cant really ahve a schema
12:24:25 sean-k-mooney by the way this coudl also be a bug
12:24:48 sean-k-mooney like 2.0 and 2.1 are ment to be identical
12:24:58 sean-k-mooney the only delta is the micoverion endpoint
12:25:02 sean-k-mooney but servers should not change
12:26:02 stephenfin they're mostly identical: look at I4c066075165f69d355a74979808fa0ad8d6546ab for example
12:26:06 stephenfin (commit 4a29486f4cf15720d09ed0ed86c47cd07f5cd14a)
12:27:23 sean-k-mooney well schduler hint need addtional properteies true alwasy
12:27:31 sean-k-mooney even in 2.100
12:27:45 sean-k-mooney that is not a 2.1 vs 2.0 compatiablity thing
12:28:11 stephenfin you're right, good point
12:28:13 sean-k-mooney schduelr hint are alwasy exstenible becuase tehy are one of the ways we provie to oeprator writing there own schduler filter
12:28:42 sean-k-mooney they are however like extra specs
12:29:00 sean-k-mooney in that we can have diffintions for the standard ones
12:31:31 sean-k-mooney stephenfin: if we only have two examples to choose between i dont think the if,then,else is much more complx the oneOF
12:31:36 sean-k-mooney so i woudl go with that for now
12:31:50 sean-k-mooney both are conditionals its just the if is more explcit
12:32:12 sean-k-mooney if you need multiple hten i woudl vote for not validaing repocnes for 2.0 at all
12:32:28 sean-k-mooney but we shoudl see what others think
12:33:00 sean-k-mooney stephenfin: we do not have an exmaple of this for 2.1+ right?
12:34:37 stephenfin No, it's not an issue for v2.1 since the soft additional properties stuff only applies to v2.0
12:35:55 sean-k-mooney stephenfin: actully i asked gemini for some ohter variation that aovid the repition let me link them
12:37:07 sean-k-mooney https://paste.opendev.org/show/b7wJJXIE4edOqlCZ3Ijj/
12:40:32 sean-k-mooney can we use unevaluatedProperties?
12:41:50 sean-k-mooney if so option 3 is an option, 1 and 2 are mostly the same it jsut comes down to an if or switch semantic to select the correct one
12:42:11 sean-k-mooney i would go with 3>2>1 in that order
12:44:25 opendevreview Stephen Finucane proposed openstack/nova-specs master: Remove v2.0 API https://review.opendev.org/c/openstack/nova-specs/+/951949
12:44:58 stephenfin sean-k-mooney: gibi: gmaan: Would be interested in hearing if there's a solid reason not to do ^
12:44:59 sean-k-mooney hum that also an approch if we do that can we add v3 as a perfect copy of 2.100 :)
12:45:25 sean-k-mooney stephenfin: please look at the previsou paste if you didnt see that howevre
12:45:56 stephenfin I suggested that before. dansmith said no
12:46:19 sean-k-mooney how about v4
12:46:26 sean-k-mooney that woudl be at least new
12:49:28 gibi stephenfin: in general I'm OK to remove v2.0 it is old as hell and right now it poses problems for us. I'm also OK to just sidestep the problem and disable response validaton in 2.0
12:49:38 sean-k-mooney my personal prefence woudl be to add v3 in 2026.1 and remove v2.0 while keeping all of 2.1-2.100 until 2028.1+
12:51:26 sean-k-mooney and in 2028.1 we woudl see if enough peopel had moved to v3 do deprection 2.1+ or not and set a deadlien for that but basiclly feature freeze that code next cycle.
12:51:59 sean-k-mooney by feature feeze i just mean no more 2.x microverions
12:52:04 sean-k-mooney not duplicate teh code
12:54:13 gibi where does the v3 and v4 idea coming from? Could we just have 2.1, remove 2.0 and that is it?
12:55:01 sean-k-mooney gibi: that was me just saying removing v2.0 specificly i think is good
12:55:17 sean-k-mooney gibi: but as i have argued for years i think its time for a v3
12:55:26 sean-k-mooney or to raise our min version
12:55:36 sean-k-mooney i know dan dose not want to od that
12:55:46 sean-k-mooney and i dont thnk we will have time to even consider a v3 this cycle
12:56:03 sean-k-mooney im just frustrated that we wont even consider doing it again
12:56:20 sean-k-mooney i think it activly makes our rest api wrose
12:56:21 gibi ahh the raising of the min version.
12:56:49 sean-k-mooney ya so if we remove 2.0 we efectivly faise ti to 2.1
12:56:55 gibi yeah here we don't need it but yes if it helps deleting code then we can consider raising the min version
12:56:59 sean-k-mooney which ithink is fine since they should be identical
12:57:56 sean-k-mooney i am supprtive of stephenfin proposal to do the minium possible min_virsion in crease form 2.0 to 2.1
12:58:16 sean-k-mooney by removeing 2.0
12:58:34 sean-k-mooney but once we are done with eventlet removal and the open api schema work
12:58:48 sean-k-mooney and service/manager policy work
12:59:18 sean-k-mooney then i think we shoudl revisit v3 or raisgin the min version to somewhere in the 2.77 ish range
12:59:47 sean-k-mooney im not pusshign for tha tnow but i thik we shoudl at least discuss how this woudl work in the next 18 months
13:00:56 gibi yeah I think it make sense to discuss it and collect a list of things we would gain out of such a change
13:02:05 sean-k-mooney stephenfin: you still have not responded on the other schema permutaions https://paste.opendev.org/show/b7wJJXIE4edOqlCZ3Ijj/ do any of those look reasonabel to you as i said my prefernce there would be 3>2>1
13:02:33 sean-k-mooney unevaluatedProperties if viable seam clean and consise
13:02:49 gibi on a totally independent topic. I just learned today that `openstack server list` triggers two independent full scatter-gather calls one for getting the instances (obviously needed) and one for getting the bdms for the instances (not so obvious). 1) https://github.com/openstack/nova/blob/68c2341b765a22b9b81894d2ff3b21fd5f8632ec/nova/api/openstack/compute/servers.py#L326
13:02:55 gibi 2)https://github.com/openstack/nova/blob/68c2341b765a22b9b81894d2ff3b21fd5f8632ec/nova/api/openstack/compute/servers.py#L342 and https://github.com/openstack/nova/blob/68c2341b765a22b9b81894d2ff3b21fd5f8632ec/nova/api/openstack/compute/views/servers.py#L497
13:03:09 stephenfin sean-k-mooney: sorry, I'd just assumed we were going to skip validation for response bodies and be done with it
13:03:27 sean-k-mooney gibi: hum thats unfortunet
13:04:11 sean-k-mooney gibi: i assume we cant pass an arge to do the join egerly
13:05:09 elodilles hi folks, i know thay you are busy with other things, but there is a small "tool" change i'd appreciate you would review and merge if possible (yeah, it's a small fix for backport validator, related to unmaintained branches :/): https://review.opendev.org/c/openstack/nova/+/949628
13:05:17 elodilles s/thay/that/
13:06:17 gibi elodilles: +2
13:06:19 sean-k-mooney looks fine to me i guess
13:06:43 sean-k-mooney it is one of the first times we have had to skip eols
13:06:54 gibi sean-k-mooney: we might, but the server/view code is entered from different places so we should only conditionally do the join if we know it will be used
13:07:05 stephenfin gibi: sean-k-mooney: thanks for the help https://github.com/stephenfin/nova/commit/2630daf37c8adf4b443c80e66139ea6285c04eae
13:07:24 stephenfin will push all those once the bulk of what's already up on gerrit has merged
13:08:17 elodilles thanks gibi sean-k-mooney o/
13:35:42 sean-k-mooney gibi: im on a call but we have someting like "expected_filds" or something like that to contole which filed are lazy loaded on the instance object
13:35:57 sean-k-mooney i dont knwo if we can use that in soem way
13:37:37 sean-k-mooney i was thinkig of https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L77C5-L101
13:46:49 opendevreview Balazs Gibizer proposed openstack/nova master: Run nova-api and -metadata in threaded mode https://review.opendev.org/c/openstack/nova/+/951957
13:48:33 gibi sean-k-mooney: yeah we can ask for the join, the problem is that there are multiple ways reaching that calls and some will use the bdms later some won't. Also the way how this is built up make is pretty hard to untangle them. It is not impossible to refactor this but I decided not to try it now

Earlier   Later