-
Notifications
You must be signed in to change notification settings - Fork 584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Discovery add history #868
Conversation
Signed-off-by: Erik Erikson <erik.erikson@gmail.com>
Signed-off-by: Erik Erikson <erik.erikson@gmail.com>
Signed-off-by: Erik Erikson <erik.erikson@gmail.com>
application/json: | ||
schema: | ||
$ref: '#/components/schemas/serviceinstance' | ||
$ref: '#/components/requestBodies/servicedeleterequest' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a requestBody on a delete is discouraged by OpenAPI for reasons discussed here. To abusively summarize the most useful piece, there is no standard about the interaction of HTTP DELETEs on to resource collections and the caching layers which may cause undesirable lack of caching on one hand and/or a use of cached resources inappropriately on the other.
e.g.
DELETE /services
[
{ id: {id_0} },
...
{ id: {id_N} },
]
Does not clear /services/{id_0}
from HTTP response caches which, unless configured not to cache, are allowed to cache and return that value following the resource's deletion using a DELETE with a request body.
As suggested and discussed, removing the changes path (i.e. HTTP history endpoint) in preference for subscriptions offering a read offset configuration. Signed-off-by: Erik Erikson <erik.erikson@gmail.com>
@duglin this has been pending the AIs from Clemens and Klaus but I am wondering if we expect those to alter this in some way? If not, we might be able to merge this and proceed by altering it as necessary? |
Signed-off-by: Erik Erikson <erik.erikson@gmail.com>
Signed-off-by: Erik Erikson <erik.erikson@gmail.com>
Approved on the 10/14 call |
Fixes #699
Proposed Changes
Release Note