On Wed, Jul 12, 2017 at 11:29 AM, Alex Xu <sou...@gmail.com> wrote: > > > 2017-07-12 9:18 GMT+08:00 Matt Riedemann <mriede...@gmail.com>: >> >> I'm looking for some broader input on something being discussed in this >> change: >> >> >> https://review.openstack.org/#/c/464280/21/nova/api/openstack/compute/services.py >> >> This is collapsing the following APIs into a single API: >> >> Old: >> >> * PUT /os-services/enable >> * PUT /os-services/disable >> * PUT /os-services/disable-log-reason >> * PUT /os-services/force-down >> >> New: >> >> * PUT /os-services >> >> With the old APIs, if you tried to enable and already enabled service, it >> was not an error. The same is you tried to disable an already disabled >> service. It doesn't change anything, but it's not an error. >> >> The question is coming up in the new API if trying to enable an enabled >> service should be a 400, or trying to disable a disabled service. The way I >> wrote the new API, those are no 400 conditions. They don't do anything, like >> before, but they aren't errors. > > > Sorry, I didn't describe clearly in the comment. > > Some of those comments about save a DB call with more conditions checks. It > means if enable a enabled service, we needn't a db call, we can just return > to the user 200 directly. > > One of those comments is about when the API user specified 'status=enabled' > and 'disabled_reason' in the request body, then we just ignore the > 'disabled_reason' and didn't save it into the db also. That sounds not > right. We should return 400 to the API user, you can't specified the > 'status=enabled' and 'disabled_reason'.
+1 for 400 in this case. > >> >> >> Looking at [1] it seems this should not be an error condition if you're >> trying to update the state of a resource and it's already at that state. >> >> I don't have a PhD in REST though so would like broader discussion on >> this. >> >> [1] http://www.restapitutorial.com/lessons/idempotency.html >> >> -- >> >> 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 > __________________________________________________________________________ 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