| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-01-31 | |||
| 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 | |
| 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 | 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:54:34 | sean-k-mooney[m] | i really dont think this is something we shoudl require a microversion change for | |
| 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 | |