Clarify how to clean-up during a failed provision/bind #353
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hey duglin! Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA. |
Closes openservicebrokerapi#348 We actually already had text to deal with this in the bind() case, it was just missing the MUST, so I added that and then copied that same text into the provision case. Net: platforms MUST send a DELETE to the brokers upon a failed provison/bind just to make sure things are cleaned up. I'll open a 2nd PR to add text talking about how the brokers SHOULD clean-up on their own w/o the need for the DELETE. Signed-off-by: Doug Davis <dug@us.ibm.com>
|
Change it from SHOULD to MUST so brokers will know what to expect. |
|
LGTM |
|
LGTM 👏 |
duglin
pushed a commit
to duglin/servicebroker
that referenced
this pull request
Oct 27, 2017
Provide more guidance for what should happen when things go wrong so we can ensure a more consistent semantics and interop. This should only be reviewed after openservicebrokerapi#353 Signed-off-by: Doug Davis <dug@us.ibm.com>
|
LGTM |
vaikas
reviewed
Oct 31, 2017
spec.md
Outdated
| @@ -817,7 +817,11 @@ $ curl http://username:password@service-broker-url/v2/service_instances/:instanc | |||
| | 409 Conflict | MUST be returned if a service instance with the same id already exists but with different attributes. The expected response body is `{}`, though the description field MAY be used to return a user-facing error message, as described in [Service Broker Errors](#service-broker-errors). | | |||
| | 422 Unprocessable Entity | MUST be returned if the Service Broker only supports asynchronous provisioning for the requested plan and the request did not include `?accepts_incomplete=true`. The expected response body is: `{ "error": "AsyncRequired", "description": "This service plan requires client support for asynchronous service operations." }`, as described below (see [Service Broker Errors](#service-broker-errors). | | |||
|
|
|||
| Responses with any other status code will be interpreted as a failure. Service brokers can include a user-facing message in the `description` field; for details see [Service Broker Errors](#service-broker-errors). | |||
| Responses with any other status code will be interpreted as a failure and a | |||
| deprovision request SHOULD be sent to the Service Broker to prevent an orphan | |||
There was a problem hiding this comment.
I'd like similar language here added to L694 saying that if a request fails (aka, we get 200 but status is failed) that deprovision will be sent too.
There was a problem hiding this comment.
Or maybe L641 is better for it. Point being that we should clarify on polling as well.
Signed-off-by: Doug Davis <dug@us.ibm.com>
|
LGTM |
2 similar comments
|
LGTM |
|
LGTM |
duglin
pushed a commit
to duglin/servicebroker
that referenced
this pull request
Oct 31, 2017
Provide more guidance for what should happen when things go wrong so we can ensure a more consistent semantics and interop. This should only be reviewed after openservicebrokerapi#353 Signed-off-by: Doug Davis <dug@us.ibm.com>
|
One more review needed |
|
LGTM |
duglin
pushed a commit
to duglin/servicebroker
that referenced
this pull request
Nov 7, 2017
Provide more guidance for what should happen when things go wrong so we can ensure a more consistent semantics and interop. This should only be reviewed after openservicebrokerapi#353 Signed-off-by: Doug Davis <dug@us.ibm.com>
duglin
pushed a commit
to duglin/servicebroker
that referenced
this pull request
Nov 8, 2017
Provide more guidance for what should happen when things go wrong so we can ensure a more consistent semantics and interop. This should only be reviewed after openservicebrokerapi#353 Signed-off-by: Doug Davis <dug@us.ibm.com>
duglin
pushed a commit
to duglin/servicebroker
that referenced
this pull request
Nov 21, 2017
Provide more guidance for what should happen when things go wrong so we can ensure a more consistent semantics and interop. This should only be reviewed after openservicebrokerapi#353 Signed-off-by: Doug Davis <dug@us.ibm.com>
duglin
pushed a commit
to duglin/servicebroker
that referenced
this pull request
Nov 21, 2017
Provide more guidance for what should happen when things go wrong so we can ensure a more consistent semantics and interop. This should only be reviewed after openservicebrokerapi#353 Signed-off-by: Doug Davis <dug@us.ibm.com>
duglin
pushed a commit
to duglin/servicebroker
that referenced
this pull request
Jan 8, 2018
Provide more guidance for what should happen when things go wrong so we can ensure a more consistent semantics and interop. This should only be reviewed after openservicebrokerapi#353 Signed-off-by: Doug Davis <dug@us.ibm.com>
duglin
pushed a commit
to duglin/servicebroker
that referenced
this pull request
Jan 9, 2018
Provide more guidance for what should happen when things go wrong so we can ensure a more consistent semantics and interop. This should only be reviewed after openservicebrokerapi#353 Signed-off-by: Doug Davis <dug@us.ibm.com>
duglin
pushed a commit
to duglin/servicebroker
that referenced
this pull request
Jan 19, 2018
Provide more guidance for what should happen when things go wrong so we can ensure a more consistent semantics and interop. This should only be reviewed after openservicebrokerapi#353 Signed-off-by: Doug Davis <dug@us.ibm.com>
duglin
pushed a commit
to duglin/servicebroker
that referenced
this pull request
Jan 22, 2018
Provide more guidance for what should happen when things go wrong so we can ensure a more consistent semantics and interop. This should only be reviewed after openservicebrokerapi#353 Signed-off-by: Doug Davis <dug@us.ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #348
We actually already had text to deal with this in the bind() case, it
was just missing the MUST, so I added that and then copied that same
text into the provision case.
Net: platforms MUST send a DELETE to the brokers upon a failed
provison/bind just to make sure things are cleaned up.
I'll open a 2nd PR to add text talking about how the brokers SHOULD
clean-up on their own w/o the need for the DELETE.
Signed-off-by: Doug Davis dug@us.ibm.com