Re: [openstack-dev] [nova] When can/should we change additionalProperties=False in GET /servers(/detail)?
On Wed, 19 Sep 2018 02:26:30 +0900 Matt Riedemann wrote > On 9/17/2018 9:41 PM, Ghanshyam Mann wrote: > > On Tue, 18 Sep 2018 09:33:30 +0900 Alex Xu wrote > > > > > That only means after 599276 we only have servers API and > > os-instance-action API stopped accepting the undefined query parameter. > > > What I'm thinking about is checking all the APIs, add json-query-param > > checking with additionalProperties=True if the API don't have yet. And > > using another microversion set additionalProperties to False, then the > > whole Nova API become consistent. > > > > I too vote for doing it for all other API together. Restricting the > > unknown query or request param are very useful for API consistency. Item#1 > > in this etherpadhttps://etherpad.openstack.org/p/nova-api-cleanup > > > > If you would like, i can propose a quick spec for that and positive > > response to do all together then we skip to do that in 599276 otherwise do > > it for GET servers in 599276. > > > > -gmann > > I don't care too much about changing all of the other > additionalProperties=False in a single microversion given we're already > kind of inconsistent with this in a few APIs. Consistency is ideal, but > I thought we'd be lumping in other cleanups from the etherpad into the > same microversion/spec which will likely slow it down during spec > review. For example, I'd really like to get rid of the weird server > response field prefixes like "OS-EXT-SRV-ATTR:". Would we put those into > the same mass cleanup microversion / spec or split them into individual > microversions? I'd prefer not to see an explosion of microversions for > cleaning up oddities in the API, but I could see how doing them all in a > single microversion could be complicated. Sounds good to me. I also do not feel like increasing microversions for every cleanup. I would like to see all cleanup(worthy cleanup) in single microversion. I have pushed the spec for that for further discussion/debate. - https://review.openstack.org/#/c/603969/ -gmann > -- > > Thanks, > > Matt > > __ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] When can/should we change additionalProperties=False in GET /servers(/detail)?
On 9/18/2018 12:26 PM, Matt Riedemann wrote: On 9/17/2018 9:41 PM, Ghanshyam Mann wrote: On Tue, 18 Sep 2018 09:33:30 +0900 Alex Xu wrote > That only means after 599276 we only have servers API and os-instance-action API stopped accepting the undefined query parameter. > What I'm thinking about is checking all the APIs, add json-query-param checking with additionalProperties=True if the API don't have yet. And using another microversion set additionalProperties to False, then the whole Nova API become consistent. I too vote for doing it for all other API together. Restricting the unknown query or request param are very useful for API consistency. Item#1 in this etherpadhttps://etherpad.openstack.org/p/nova-api-cleanup If you would like, i can propose a quick spec for that and positive response to do all together then we skip to do that in 599276 otherwise do it for GET servers in 599276. -gmann I don't care too much about changing all of the other additionalProperties=False in a single microversion given we're already kind of inconsistent with this in a few APIs. Consistency is ideal, but I thought we'd be lumping in other cleanups from the etherpad into the same microversion/spec which will likely slow it down during spec review. For example, I'd really like to get rid of the weird server response field prefixes like "OS-EXT-SRV-ATTR:". Would we put those into the same mass cleanup microversion / spec or split them into individual microversions? I'd prefer not to see an explosion of microversions for cleaning up oddities in the API, but I could see how doing them all in a single microversion could be complicated. Just an update on https://review.openstack.org/#/c/599276/ - the change is approved. We left additionalProperties=True in the GET /servers(/detail) APIs for consistency with 2.5 and 2.26, and for expediency in just getting the otherwise pretty simple change approved. -- Thanks, Matt __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] When can/should we change additionalProperties=False in GET /servers(/detail)?
On 9/17/2018 9:41 PM, Ghanshyam Mann wrote: On Tue, 18 Sep 2018 09:33:30 +0900 Alex Xu wrote > That only means after 599276 we only have servers API and os-instance-action API stopped accepting the undefined query parameter. > What I'm thinking about is checking all the APIs, add json-query-param checking with additionalProperties=True if the API don't have yet. And using another microversion set additionalProperties to False, then the whole Nova API become consistent. I too vote for doing it for all other API together. Restricting the unknown query or request param are very useful for API consistency. Item#1 in this etherpadhttps://etherpad.openstack.org/p/nova-api-cleanup If you would like, i can propose a quick spec for that and positive response to do all together then we skip to do that in 599276 otherwise do it for GET servers in 599276. -gmann I don't care too much about changing all of the other additionalProperties=False in a single microversion given we're already kind of inconsistent with this in a few APIs. Consistency is ideal, but I thought we'd be lumping in other cleanups from the etherpad into the same microversion/spec which will likely slow it down during spec review. For example, I'd really like to get rid of the weird server response field prefixes like "OS-EXT-SRV-ATTR:". Would we put those into the same mass cleanup microversion / spec or split them into individual microversions? I'd prefer not to see an explosion of microversions for cleaning up oddities in the API, but I could see how doing them all in a single microversion could be complicated. -- Thanks, Matt __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] When can/should we change additionalProperties=False in GET /servers(/detail)?
On Tue, 18 Sep 2018 09:33:30 +0900 Alex Xu wrote > That only means after 599276 we only have servers API and os-instance-action > API stopped accepting the undefined query parameter. > What I'm thinking about is checking all the APIs, add json-query-param > checking with additionalProperties=True if the API don't have yet. And using > another microversion set additionalProperties to False, then the whole Nova > API become consistent. I too vote for doing it for all other API together. Restricting the unknown query or request param are very useful for API consistency. Item#1 in this etherpad https://etherpad.openstack.org/p/nova-api-cleanup If you would like, i can propose a quick spec for that and positive response to do all together then we skip to do that in 599276 otherwise do it for GET servers in 599276. -gmann > Jay Pipes 于2018年9月18日周二 上午4:07写道： > __ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > On 09/17/2018 03:28 PM, Matt Riedemann wrote: > > This is a question from a change [1] which adds a new changes-before > > filter to the servers, os-instance-actions and os-migrations APIs. > > > > For context, the os-instance-actions API stopped accepting undefined > > query parameters in 2.58 when we added paging support. > > > > The os-migrations API stopped allowing undefined query parameters in > > 2.59 when we added paging support. > > > > The open question on the review is if we should change GET /servers and > > GET /servers/detail to stop allowing undefined query parameters starting > > with microversion 2.66 [2]. Apparently when we added support for 2.5 and > > 2.26 for listing servers we didn't think about this. It means that a > > user can specify a query parameter, documented in the API reference, but > > with an older microversion and it will be silently ignored. That is > > backward compatible but confusing from an end user perspective since it > > would appear to them that the filter is not being applied, when it fact > > it would be if they used the correct microversion. > > > > So do we want to start enforcing query parameters when listing servers > > to our defined list with microversion 2.66 or just continue to silently > > ignore them if used incorrectly? > > > > Note that starting in Rocky, the Neutron API will start rejecting > > unknown query parameteres [3] if the filter-validation extension is > > enabled (since Neutron doesn't use microversions). So there is some > > precedent in OpenStack for starting to enforce query parameters. > > > > [1] https://review.openstack.org/#/c/599276/ > > [2] > > > https://review.openstack.org/#/c/599276/23/nova/api/openstack/compute/schemas/servers.py > > > > > [3] > > https://docs.openstack.org/releasenotes/neutron/rocky.html#upgrade-notes > > My vote would be just change additionalProperties to False in the 599276 > patch and be done with it. > > Add a release note about the change, of course. > > -jay > > __ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] When can/should we change additionalProperties=False in GET /servers(/detail)?
That only means after 599276 we only have servers API and os-instance-action API stopped accepting the undefined query parameter. What I'm thinking about is checking all the APIs, add json-query-param checking with additionalProperties=True if the API don't have yet. And using another microversion set additionalProperties to False, then the whole Nova API become consistent. Jay Pipes 于2018年9月18日周二 上午4:07写道： > On 09/17/2018 03:28 PM, Matt Riedemann wrote: > > This is a question from a change [1] which adds a new changes-before > > filter to the servers, os-instance-actions and os-migrations APIs. > > > > For context, the os-instance-actions API stopped accepting undefined > > query parameters in 2.58 when we added paging support. > > > > The os-migrations API stopped allowing undefined query parameters in > > 2.59 when we added paging support. > > > > The open question on the review is if we should change GET /servers and > > GET /servers/detail to stop allowing undefined query parameters starting > > with microversion 2.66 [2]. Apparently when we added support for 2.5 and > > 2.26 for listing servers we didn't think about this. It means that a > > user can specify a query parameter, documented in the API reference, but > > with an older microversion and it will be silently ignored. That is > > backward compatible but confusing from an end user perspective since it > > would appear to them that the filter is not being applied, when it fact > > it would be if they used the correct microversion. > > > > So do we want to start enforcing query parameters when listing servers > > to our defined list with microversion 2.66 or just continue to silently > > ignore them if used incorrectly? > > > > Note that starting in Rocky, the Neutron API will start rejecting > > unknown query parameteres [3] if the filter-validation extension is > > enabled (since Neutron doesn't use microversions). So there is some > > precedent in OpenStack for starting to enforce query parameters. > > > > [1] https://review.openstack.org/#/c/599276/ > > [2] > > > https://review.openstack.org/#/c/599276/23/nova/api/openstack/compute/schemas/servers.py > > > > [3] > > https://docs.openstack.org/releasenotes/neutron/rocky.html#upgrade-notes > > My vote would be just change additionalProperties to False in the 599276 > patch and be done with it. > > Add a release note about the change, of course. > > -jay > > __ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] When can/should we change additionalProperties=False in GET /servers(/detail)?
On 9/17/2018 3:06 PM, Jay Pipes wrote: My vote would be just change additionalProperties to False in the 599276 patch and be done with it. Well, it would be on a microversion boundary so the user would be opting into this stricter validation, but that's the point of microversions. So my custom API extension that handles GET /servers?bestpet=cats will continue to work as long as I'm using microversion < 2.66. -- Thanks, Matt __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] When can/should we change additionalProperties=False in GET /servers(/detail)?
On 09/17/2018 03:28 PM, Matt Riedemann wrote: This is a question from a change [1] which adds a new changes-before filter to the servers, os-instance-actions and os-migrations APIs. For context, the os-instance-actions API stopped accepting undefined query parameters in 2.58 when we added paging support. The os-migrations API stopped allowing undefined query parameters in 2.59 when we added paging support. The open question on the review is if we should change GET /servers and GET /servers/detail to stop allowing undefined query parameters starting with microversion 2.66 [2]. Apparently when we added support for 2.5 and 2.26 for listing servers we didn't think about this. It means that a user can specify a query parameter, documented in the API reference, but with an older microversion and it will be silently ignored. That is backward compatible but confusing from an end user perspective since it would appear to them that the filter is not being applied, when it fact it would be if they used the correct microversion. So do we want to start enforcing query parameters when listing servers to our defined list with microversion 2.66 or just continue to silently ignore them if used incorrectly? Note that starting in Rocky, the Neutron API will start rejecting unknown query parameteres [3] if the filter-validation extension is enabled (since Neutron doesn't use microversions). So there is some precedent in OpenStack for starting to enforce query parameters. [1] https://review.openstack.org/#/c/599276/ [2] https://review.openstack.org/#/c/599276/23/nova/api/openstack/compute/schemas/servers.py [3] https://docs.openstack.org/releasenotes/neutron/rocky.html#upgrade-notes My vote would be just change additionalProperties to False in the 599276 patch and be done with it. Add a release note about the change, of course. -jay __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [nova] When can/should we change additionalProperties=False in GET /servers(/detail)?
This is a question from a change [1] which adds a new changes-before filter to the servers, os-instance-actions and os-migrations APIs. For context, the os-instance-actions API stopped accepting undefined query parameters in 2.58 when we added paging support. The os-migrations API stopped allowing undefined query parameters in 2.59 when we added paging support. The open question on the review is if we should change GET /servers and GET /servers/detail to stop allowing undefined query parameters starting with microversion 2.66 [2]. Apparently when we added support for 2.5 and 2.26 for listing servers we didn't think about this. It means that a user can specify a query parameter, documented in the API reference, but with an older microversion and it will be silently ignored. That is backward compatible but confusing from an end user perspective since it would appear to them that the filter is not being applied, when it fact it would be if they used the correct microversion. So do we want to start enforcing query parameters when listing servers to our defined list with microversion 2.66 or just continue to silently ignore them if used incorrectly? Note that starting in Rocky, the Neutron API will start rejecting unknown query parameteres [3] if the filter-validation extension is enabled (since Neutron doesn't use microversions). So there is some precedent in OpenStack for starting to enforce query parameters. [1] https://review.openstack.org/#/c/599276/ [2] https://review.openstack.org/#/c/599276/23/nova/api/openstack/compute/schemas/servers.py [3] https://docs.openstack.org/releasenotes/neutron/rocky.html#upgrade-notes -- Thanks, Matt __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
