| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-01-31 | |||
| 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 | |
| 03:01:04 | gmann | even rejecting unknown and invalid query param (which we ignored silently in code) was also changed with microversion so that we do not break users | |
| 03:01:48 | gmann | I am not saying that is good and should not be fixed. it should be but with microversion | |
| 03:03:14 | sean-k-mooney[m] | i guess with my downstream hat on i would be much less willing to supprot this. upstream we might be able to allow invalid input but form a downstream perspecitive i dont think i would accpete this form a customer bug. the api contract exits for a reason and not enforcing that contract exposes us to suppporting thing we never intended that is harmful to the healt of the product in my view | |
| 03:04:29 | gmann | my 1 month old saying to stop arguing on APIs :) its time to feed him. :P | |
| 03:04:41 | sean-k-mooney[m] | :) | |
| 03:04:52 | sean-k-mooney[m] | go do that | |
| 03:05:35 | sean-k-mooney[m] | but if we cant fix this without a microverion or validation middleware i think we have to reconsider raising the min microversion to fix this | |
| 08:04:56 | gibi | the more I think about API contract the more I feel that we are on the wrong path. Saying that we should not change the behavior of our APIs without a microversion could be abused to the extent that forbid any kind of change of the implementation behind the API. | |
| 08:06:52 | gibi | say we have a placement sql bug that breaks resource acconting today in a certain situation. As this is how the service works today one can say that a client might depend on the buggy behavior of the resource accounting hence we cannot fix the accounting outside of a microversion so placement servers out on the field will have the buggy behavior until everybody upgrades to future mastyer | |
| 08:07:17 | gibi | so imagine two customers | |
| 08:07:41 | gibi | i) customer depending on the buggy behavior and getting a stable bugfix would break their usage of placement | |