Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-01-31
17:17:04 gibi as she get B so far but not any more
17:17:22 gibi melwitt: yeah, it seems 3 of us has 3 different area of concern :)
17:17:27 gibi fun :0
17:17:29 gibi ;)
17:17:32 melwitt yeah 😂
17:17:44 gmann gibi: well, we at least does break them in term of return code. and say required=A&required=B we meant for return A and B and we fixed it now.
17:18:01 gmann but returning 400 for them break them as they get something and then they get error
17:18:44 gibi gmann: I'm pretty sure if the relied on gettin RPs with B traits only and now getting empty result as we returning onyl RPs with both A and B will break tem
17:18:47 gibi them
17:18:48 gmann I mean we fix the code to make it return A and B is ok but making them error is issue
17:19:33 gmann gibi: we had similar (not exactly same ) issue in nova query param for silently ignoring few/unknown also and we could not change it without microversion
17:20:44 gibi OK, I'm dropping the bugfix from the patchseries of microversion 1.39. I still intend to land 1.39 this cycle. I don't have such mandate for the bugfix itself. so I prioritize
17:20:45 gmann gibi: sure, in that case we can just leave the current behavior as it is and not breaking anything and say "new microversion gives correct behavior"
17:21:27 gmann I am if we want to change for current microversion then we "make it more correct is fine but rejecting the request is not"
17:21:36 melwitt wait, I don't understand why the bug fix can't go in 1.39 if 1.39 is not released yet?
17:21:57 gmann I think gibi saying bug fix for older microversion also
17:22:08 gibi melwitt: the bug will disappera in 1.39
17:22:20 gibi melwitt: but I will not try to fix it in <1.39 now
17:22:28 gibi as it seem we cannot agree
17:22:59 melwitt ah ok
17:23:00 gibi 1.39 alway planned to allow repeating the required query param
17:23:17 gibi as in: needs to be repeated for (A or B) and (C or D) case
17:24:11 melwitt ok, so repeated was not officially supported < 1.39. sorry I had missed that
17:24:44 gmann yeah, it was always nor documented neither we knew how it work until gibi found it?
17:24:47 gibi melwitt: <1.39 there was not defined behavior for repeat
17:25:08 gibi the code happened to pares the last repetition
17:25:12 gibi parse
17:25:15 gibi and ignore the rest
17:25:49 melwitt gotcha.. thanks
17:28:48 opendevreview Balazs Gibizer proposed openstack/placement master: Extra tests around required traits https://review.opendev.org/c/openstack/placement/+/825846
17:28:49 opendevreview Balazs Gibizer proposed openstack/placement master: Refactor trait normalization https://review.opendev.org/c/openstack/placement/+/825847
17:28:49 opendevreview Balazs Gibizer proposed openstack/placement master: Extend the RP db query to support any-traits https://review.opendev.org/c/openstack/placement/+/825848
17:28:50 opendevreview Balazs Gibizer proposed openstack/placement master: DB layer should only depend on trait id not names https://review.opendev.org/c/openstack/placement/+/826490
17:28:50 opendevreview Balazs Gibizer proposed openstack/placement master: Enhance doc of _get_trees_with_traits https://review.opendev.org/c/openstack/placement/+/825780
17:28:51 opendevreview Balazs Gibizer proposed openstack/placement master: Extend the RP tree DB query to support any-traits https://review.opendev.org/c/openstack/placement/+/825849
17:28:51 opendevreview Balazs Gibizer proposed openstack/placement master: Add any-traits support for listing resource providers https://review.opendev.org/c/openstack/placement/+/826491
17:28:52 opendevreview Balazs Gibizer proposed openstack/placement master: Add any-traits support for allocation candidates https://review.opendev.org/c/openstack/placement/+/826492
17:28:52 opendevreview Balazs Gibizer proposed openstack/placement master: Remove unused compatibility code https://review.opendev.org/c/openstack/placement/+/826493
17:28:54 opendevreview Balazs Gibizer proposed openstack/placement master: Add microversion 1.39 to support any-trait queries https://review.opendev.org/c/openstack/placement/+/826719
17:29:23 melwitt dansmith, bauzas: just a fyi, oslo.limit got a new release so I cleaned up the unified limits set to use it and removed the haxx
17:30:12 gibi sean-k-mooney, gmann, melwitt, efried: I restored the patch series of microversion 1.39 not to change the behavior of the repeated required param handling in < 1.39 microversion https://review.opendev.org/q/topic:any-traits-support
17:30:42 gibi so < 1.39 if the required param is repeated only the _last_ instance is parsed a rest is ignored
17:30:46 bauzas melwitt: cool, as I said, you're my next priority once I'm done with trying to work on Tempest :p
17:31:40 melwitt k :)
17:34:04 dansmith melwitt: nice
18:07:23 sean-k-mooney gibi: ack
18:20:57 opendevreview Rajat Dhasmana proposed openstack/nova master: WIP: Add support for volume backed server rebuild https://review.opendev.org/c/openstack/nova/+/820368
18:27:49 opendevreview Rajat Dhasmana proposed openstack/python-novaclient master: WIP: Add parameter to rebuild boot volume https://review.opendev.org/c/openstack/python-novaclient/+/827163
18:38:08 opendevreview Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053
18:45:38 opendevreview Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053
18:49:53 opendevreview Merged openstack/nova master: Add nova-ovs-hybrid-plug job https://review.opendev.org/c/openstack/nova/+/817303
19:35:27 opendevreview Merged openstack/nova master: Deprecate the powervm driver https://review.opendev.org/c/openstack/nova/+/819366
19:42:19 chateaulav can i get a second look on https://review.opendev.org/c/openstack/nova/+/822053, not sure what happened but it seems to be wanting to merge... i think im stuck and not sure what i broke
19:56:23 melwitt chateaulav: it show it's in merge conflict, should just need a rebase
19:57:01 melwitt *shows
19:57:31 chateaulav melwitt: yeah, tried, i think i got it. had to hard reset the branch. gonna add all the changes back and see if it resolves now
19:57:33 chateaulav thanks
20:04:04 melwitt ack
20:43:07 opendevreview Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053
20:48:44 opendevreview Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053
20:50:57 opendevreview Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053
20:53:19 opendevreview Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053
20:55:36 opendevreview Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053
21:11:26 opendevreview Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053
21:17:00 opendevreview Jonathan Race proposed openstack/nova master: Adds Pick guest CPU architecture based on host arch in libvirt driver support https://review.opendev.org/c/openstack/nova/+/822053
#openstack-nova - 2022-02-01
00:16:52 gmann gibi: ack
02:30:43 gmann gibi: sean-k-mooney[m] melwitt just tested that nova API query param also allow duplicate query param and take latest one. I remember somewhere we convert that to python dict that is why the last one is considered.
02:31:13 melwitt well that's fun
02:31:52 sean-k-mooney[m] there are some query parmaters that we all to be repeated like field that can be used to generate an array
02:32:19 sean-k-mooney[m] but unless we expressly document that as a supported usecase in teh api ref using it should be considerd a bug
02:32:49 sean-k-mooney[m] if its not in the api-ref or in the orginal spec it is not part of the public api contract
02:33:16 sean-k-mooney[m] gmann which nova api query parmater were you testing?
02:33:26 gmann list server
02:33:40 gmann limit filter
02:37:54 sean-k-mooney[m] no user should ever depend on the last instance of a parmater being used in the nova api we are free to change that at any point unless there is api documenation that expressless states that for any given parmater.
02:39:39 sean-k-mooney[m] put another way i do not think we should ever accpeat a bug if we change that behaivor without a microversion as it is an out of contract use fo the api and governed by the micorversion contract as a result
02:48:42 sean-k-mooney[m] ack i would consider depending on that behavior to be unsupported, https://docs.openstack.org/api-ref/compute/?expanded=list-servers-detail#list-servers does not declare you are allowed to repate the args as part of the filter support
02:49:46 sean-k-mooney[m] we should be retrunnign a 400 but we are not validateing that. if we change the behavior in the future it should not require a microversion as this is not part of the api specification today
02:52:35 gmann gibi: melwitt sean-k-mooney[m] we validate it by converting it to set which will add multiple passed value in query param - https://github.com/openstack/nova/blob/master/nova/api/validation/__init__.py#L176
02:52:41 gmann which is good.
02:52:54 gmann but here this guy just pick latest one https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L179
02:53:34 gmann sean-k-mooney[m]: additional param in query param was also not written behavior in api-ref but we change that with microversion so that we do not break users
02:54:13 sean-k-mooney[m] i really think this raised to the level of a v3 api change if we dont reject it without a microversion change
02:54:34 sean-k-mooney[m] i really dont think this is something we shoudl require a microversion change for
02:54:34 gmann I consider API usage as what code allow than what is there in api-ref. api-ref are not complete set of 'how not to use' its just 'how to use'
02:55:20 sean-k-mooney[m] to me the is a unaccpable burden of mantaince to contineu to supprot. the api is not defiend by the implemantion its defiend by the specification and this is not part of the specificaiont
02:55:21 gmann it is like we allowed our API to be used that wrong way and for many users it was working that as successful case.
02:55:56 gmann sean-k-mooney[m]: not our API :) api -ref was written based on what implementation was.
02:56:33 gmann we try to match that but if api-ref mention something and code does not behave that way then yes we consider as bug
02:57:03 gmann but if api-ref which is not complete does not mention anything does not mean code is wrong and we can change that without versioning
02:57:34 gmann in past we used to document that as known bug/behavior and fix in new microversion
02:57:51 sean-k-mooney[m] i disagree if the api and spec for the feature do not refernce the behaivor it is not part of the public contract
02:58:32 sean-k-mooney[m] i dont think we should leak the impleation into the public contract
02:58:40 gmann it should if we have designed our API that way but it is not. it was always what implementation was and that is being used.
02:58:52 sean-k-mooney[m] we did design our apis that way
02:59:02 sean-k-mooney[m] its why we require a spec for all api changes
02:59:40 sean-k-mooney[m] the really issue i guess is some fo the apis are form pre microversion and did not have a spec
02:59:59 gmann that is fine. I am saying for old and existing behavior. that is why microversion was introduced. for new APis/feature I agree as per spec driven.
03:00:01 sean-k-mooney[m] for example the regex support today in list servers is directly tied to the db you use
03:00:54 sean-k-mooney[m] i really think it would reduce the quality of our software to not fix what in my mind is broken validation code

Earlier   Later