Call out that extra fields in requests and responses are allowed

[duglin]

This change will make it clear that request and responses that have HTTP bodies MAY have additional fields specified by the sender.

Signed-off-by: Doug Davis dug@us.ibm.com

[cfdreddbot]

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.

[mattmcneeney]

I'm happy with this, but we could keep the spec shorter by having this in some generic section at the top somewhere?

[duglin]

I thought about that and about how we've done this in other specs. Normally what I've seen is that the spec would put "..." in the normative pseudo schema that shows what the messages look like on the wire and where extensions can go. And then in a single spot near the top of the spec it says that "... means 'extensions go here'". However, we don't have pseudo schema - we just have a table listing the properties and then examples. So, I felt kind of stuck.

As an alternative, we could just add something like:

---

For each operation defined in this specification, there are a set of properties defined that make up the request and response messages. Implementations MAY choose to include additional properties, as extensions, in those messages. Those extension property names MUST NOT conflict with properties defined by this specification. Receivers of these messages MAY ignore any extensions unless some out of band agreement has been made.

---

The only issue with this is that we don't have the ability to say exactly where extensions can go - it basically says any JSON object can have them. That might be ok, we'd just need to check each one.

[duglin]

@mattmcneeney WDYT?

[mattmcneeney]

@duglin I prefer having something at the top, but I don't love the term "extensions" here, especially if we're going to use it as part of the "actions" work that is going on. Can't we just say "Implementations MAY choose to include additional properties in requests and responses."

[duglin]

Been thinking about this and regardless of whether we want to put additional (unknown) properties in an "extensions" wrapper, I don't think we can ban the use of extensions outside of the wrapper - at least not w/o breaking backwards compatibility. So, rather than having two ways of doing the same thing, I'm thinking that we should: 1 - codify in the spec what people are already doing today - allow extra properties 2 - add a v3 item to have a more clear "extension" policy defined. For example, in some specs they define a well-known place for extensions and then any unknown properties outside of that place are considered fatal errors. This gives you the ability to have a sender to know for sure that the message will not be processed if certain extensions are not supported - as well as support the (normal) optional extensions idea.

WDYT?

[n3wscott]

I agree with @mattmcneeney for just a all encompassing mention that this is fine to do. Adding this language in each object definition seems redundant.

[duglin]

ok - put it in one spot- see whatcha think

[duglin]

Reviews needed

[n3wscott]

Thanks for that change, it looks awesome now.

LGTM

[fmui]

LGTM

[duglin]

@pmorie see if this aligns with what we talked about earlier today

[pmorie]

I would like to avoid having this merged via additional LGTM before we can discuss this as a group; let's discuss this on the call tomorrow.

[duglin]

ok- updated per today's call.

ping @pmorie @n3wscott @kibbles-n-bytes

[n3wscott]

I don't think you should be deleting comments @duglin. I feel like that is an overstep of the github admin power. @pmorie

[duglin]

I didn't want to people get confused because the comment was old and based on old text. I'll skip doing that next time

On Mar 21, 2018, at 12:24 PM, Scott Nichols notifications@github.com wrote:

I don't think you should be deleting comments @duglin https://github.com/duglin. I feel like that is an overstep of the github admin power. @pmorie https://github.com/pmorie — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openservicebrokerapi/servicebroker/pull/436#issuecomment-375004634, or mute the thread https://github.com/notifications/unsubscribe-auth/AB2sXyPVAKnGQUsVNlH8to6ibbeKtL7Lks5tgn7SgaJpZM4R02kc.

[duglin]

updated section title

[duglin]

Reviews needed

[fmui]

LGTM

[n3wscott]

LGTM

[mattmcneeney]

LGTM

[duglin]

hmm @n3wscott isn't in the PA list of people for some reason - very odd

[duglin]

I take that back - he is. No idea what's going on. I think we may need to manually count them on this one. I count 3 so far.

[duglin]

rebased to see if that helps

[n3wscott]

LGTM

[n3wscott]

Hey it is working again! I think pullapprove was having some issues.

[fmui]

LGTM

[vaikas]

LGTM