Support for service binding response schemas #96
Conversation
|
ParameterSchemas (https://github.com/luksa/go-open-service-broker-client/blob/a88e490b63a03147c6bf150669ad43f2d7725400/v2/types.go#L101) should be renamed. Any suggestions? |
|
@luksa why do you say they should be renamed? |
v2/get_catalog.go
Outdated
| for ii := range catalogResponse.Services { | ||
| for jj := range catalogResponse.Services[ii].Plans { | ||
| catalogResponse.Services[ii].Plans[jj].ParameterSchemas = nil | ||
| } | ||
| } | ||
| } else if !c.APIVersion.AtLeast(Version2_14()) { |
There was a problem hiding this comment.
We should make this dependent on alpha features being enabled until openservicebrokerapi/servicebroker#392 is merged and released in the spec, so this should be else if c.EnableAlphaFeatures { instead.
There was a problem hiding this comment.
Shouldn't I add !c.EnableAlphaFeatures as an additional condition? There's a similar if statement a few lines up, which checks if the version is less than 2.13 and removes the entire ParameterSchemas field. AFAICT that's because the client is supposed to only return fields that are part of the specific version of the OSBAPI spec the client is configured to use.
There was a problem hiding this comment.
Nevermind. I get what you mean. Fixed it.
|
@pmorie The |
Excellent point. Let's do this - I'd like to cut a release of this repo before we rename the schema field, which I think we should do in a separate PR. |
luksa
force-pushed
the
binding_response_schema
branch
from
a88e490 to
155663a
Compare
luksa
force-pushed
the
binding_response_schema
branch
4 times, most recently
from
e539663 to
8271422
Compare
v2/types.go
Outdated
| // update of an API resource, and a schema for the credentials returned by the | ||
| // broker | ||
| type RequestResponseSchema struct { | ||
| *InputParametersSchema |
There was a problem hiding this comment.
Seems like we should embed the type rather than a pointer to it - WDYT?
luksa
force-pushed
the
binding_response_schema
branch
from
8271422 to
59cb2b6
Compare
| type RequestResponseSchema struct { | ||
| InputParametersSchema | ||
| // The schema definition for the broker's response to the bind request. | ||
| Response interface{} `json:"response,omitempty"` |
There was a problem hiding this comment.
Perhaps call the object ResponseSchema?
There was a problem hiding this comment.
OH oh oh... I take it back.
There was a problem hiding this comment.
This looks good to me, I am just having brain hemorrhaging from switching between c++ and go...
There was a problem hiding this comment.
You mean rename the field Response to ResponseSchema? Not sure if that makes sense - the field name is aligned with the Parameters field in InputParametersSchema and matches the JSON field name.
But what I would love is if I could rename InputParametersSchema to RequestSchema. Then, we'd have RequestSchema.Parameters and RequestResponseSchema.Response.
There was a problem hiding this comment.
I am sorry, I was 1000% mistaken. I was reading it as the object type Response, but it is of type interface{}.
What you have is great.
|
LGTM |
No description provided.