Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-01-31
17:06:51 gibi gmann: but sean suggest to make that 400 instead there too
17:06:52 gmann I too initially until I checked in doc :)
17:07:14 gibi sorry for not being clear about that
17:07:18 gmann gibi: +1 on making 400 there which will be a clear usage instead of supporting multiple way
17:07:27 melwitt I think translation is fine in 1.39
17:07:29 gmann I mean with mivroversion
17:07:45 melwitt I was thinking what to do for <= 1.38
17:08:10 gmann melwitt: but then we end up supporting 1. required=A&required=B 2. required=in:A,B 3. required=A,B
17:08:28 gibi to note that is the easiest to support from code perspective :)
17:08:40 gibi to reject 3. I have to add extra logic
17:08:41 gmann IMO, in new microversion we can make first two to avoid confusion
17:09:08 melwitt what's wrong with supporting repeated and non repeated in 1.39?
17:09:15 gibi also note that repeating required=in: is needed to be able to express (A or B) and (C or D)
17:09:43 gmann anyways with new microversion, we can see what all to support. but my only concern is for older microversion we do not change anything what it is today
17:10:05 gmann and document in api-ref that required=A&required=B is unknown behavior until v1.39
17:10:19 gibi gmann: do we have a description that defines the line between API behavior (that is unfixable in a bugfix) and non API behavior that fixabelk in a bugfix
17:10:22 gibi ?
17:10:24 melwitt even honoring valid traits? I think at the very least it should be changed to honor the valid traits
17:12:54 gmann melwitt: yeah, for invalid I am ok. my concern is on required=VALID_A&required=VALID_B return VALID_B response today
17:13:54 gmann gibi: that is always debatable :). But IMO anything working successfully today even with wrong usage of API is what we should not change. in this case required=A&required=B, user gets 'B' response is successfully case.
17:14:24 gibi gmann: does successfull means http 2xx response?
17:14:32 gmann and we do not know user expectation is only to get B and by mistake they added A too in early query param
17:14:39 gmann gibi: ^^
17:14:54 gmann this case ^^ not just 200
17:15:30 melwitt hm, ok. I guess we disagree there, I think that it should be fixed to return VALID_A and VALID_B even for the current microversion
17:15:57 gmann melwitt: I am fine with that but not with returning 400 in this case for current microversion
17:15:58 gibi melwitt: I agree that we disagree :) and I would simply reject repeated params in the current microverison
17:16:25 gibi gmann: I don't get your last point to melwitt
17:16:50 gibi gmann: if we start returning A and B response for required=A&required=B but the user wants to get B only then it is a behavior change
17:16:59 melwitt fwiw I'm not as concerned about the 400, my main concern is honoring the valid traits in the current microversion
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: Extend the RP db query to support any-traits https://review.opendev.org/c/openstack/placement/+/825848
17:28:49 opendevreview Balazs Gibizer proposed openstack/placement master: Refactor trait normalization https://review.opendev.org/c/openstack/placement/+/825847
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: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: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: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:52 opendevreview Balazs Gibizer proposed openstack/placement master: Remove unused compatibility code https://review.opendev.org/c/openstack/placement/+/826493
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: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

Earlier   Later