cloudevents / spec

CloudEvents Specification
https://cloudevents.io
Apache License 2.0
5.07k stars 583 forks source link

Temporarily cut some attributes for 0.1? #163

Closed inlined closed 6 years ago

inlined commented 6 years ago

We've spent a lot of time to harden some of the attributes: source, eventType, cloudEventsVersion, eventID, eventTime, and data. I'm really happy to stand by all of these.

Some of the other fields really haven't had the same attention and I wonder if we should have the opportunity to reintroduce them with proper scrutiny after KubeCon. Unless anyone plans to actually use these fields to achieve any KubeCon demo, I suspect we can afford to temporarily cut the following:

inlined commented 6 years ago

@duglin Can I get a 0.1 label? I'm glad to spin up PRs for this bug that each cut a field so people can discuss the merits or I can let everyone discuss here.

duglin commented 6 years ago

@inlined I've added this to the agenda for the call this week - then the group can decide on whether to include it in v0.1 or not.

My 2 cents:

contentType is needed if people put the entire CloudEvent in the HTTP body and not just the data. While less critical than contentType, I think not having a spot for extensions (or not talking about them) could send the wrong message about the flexibility we want to offer.

schemaURL and eventTypeVersion are less clear to me - and I do wonder why they can't be encoded in the contentType.

inlined commented 6 years ago

contentType is needed if people put the entire CloudEvent in the HTTP body and not just the data.

Honestly it seems sane that if you want an XML body then you should use a (yet to be defined) XML encoding for the Cloud Event:

curl -X POST endpoint \
  -H "Content-Type: application/xml"
  -d \
 '<ce:event xmlns:ce="https://cloudevent.com/v1.0/spec.xml">
    <ce:eventType value="aws:s3:change"/>
    <ce:eventTime value="2018-06-05T12:00:00Z"/>
    <ce:data><message>The user said, "Hello, world!"</message></ce:data>
 </ce:event>'

This is much cleaner than allowing:

curl -X POST endpoint \
  -H "Content-Type: application/json"
  -d \
 '{
    "eventType": "aws:s3:change",
    "eventTime": "2018-06-05T12:00:00Z",
    "contentTye": "application/xml",
    "data": "<message>The user said, &quot;Hello, world!&quot</message>",
}'

And if an XML gateway wanted to forward

curl -X POST endpoint \
  -H "Content-Type: application/xml"
  -d \
 '<ce:event xmlns:ce="https://cloudevent.com/v1.0/spec.xml">
    <ce:eventType value="aws:s3:change"/>
    <ce:eventTime value="2018-06-05T12:00:00Z"/>
    <ce:data ce:contentType="application/json">{"hello": "world"}</ce:data>
 </ce:event>'

to a JSON endpoint, would the JSON value of data be "{\"hello\":\"world\"}" or {"hello":"world"}? It seems like the spec would be simpler if we disallowed double/switched encoding. I have not yet heard of a real need for it.

inlined commented 6 years ago

I think not having a spot for extensions (or not talking about them) could send the wrong message about the flexibility we want to offer.

What if we explicitly put extensions in our roadmap?

clemensv commented 6 years ago

contentType is referenced in https://github.com/cloudevents/spec/pull/158, https://github.com/cloudevents/spec/pull/157, https://github.com/cloudevents/spec/pull/148, and https://github.com/cloudevents/spec/pull/155 therefore I am strongly opposing cutting that field, which doesn't only declare the data format, but might actually declare the schema by reference to a well-defined media type that uses the +json extension.

I am generally opposed to cutting stuff "temporarily" just because we're coming up onto a milestone. Each of the attributes currently in the spec has had some level of discussion around it, and either we drop them for good or we change them or we keep them, but I'm principally opposed to shelving a bunch of stuff just because. The discussed fields are all optional, which means that if you aren't comfortable with them, simply don't use them in your 0.1 implementation.

Regarding extensions: I agree with this property bag being poorly defined. Nevertheless, I think we need such a property bag for vendor extensibility and whether we call that headers, properties, or extensions is ultimately not all that important to us and therefore the field can stand for 0.1. But we want exactly one such property bag and not one to go along with each property, eg. we're opposed to a special property bag scoped to "source" https://github.com/cloudevents/spec/pull/138

duglin commented 6 years ago

To clarify my previous comment... while I'm not 100% clear on whether we should keep/remove some of the attributes, I'm ok with keeping them for v0.1 since I think they've been there from the start and removing them now might feel a bit rushed as we come up on our first milestone target. I don't see any harm in resolving their presence in the spec during v0.2.

duglin commented 6 years ago

We plan to discuss this (briefly) during this week's call - so if you have any comments please add them here.

inlined commented 6 years ago

We stated in last week's meeting that the "core" features will become much more solid once 0.1 is ratified. If we haven't had a chance to even look at the use case of a feature yet, I would suggest we don't know whether there would be wildly breaking changes in the future. If we can't make any promise to the stability of a feature then I don't think we should encourage early adopters to try them out.

duglin commented 6 years ago

We stated in last week's meeting that the "core" features will become much more solid once 0.1 is ratified.

I can't recall if that was said or not, but IMO going to v0.1 just gives us a name/tag that we can all point to as we write code for the interop event at CNCF/KubeCon. Nothing is more, or less, set in stone just because we reached the first milestone. IOW, everything in the spec can still change if the group wants to change it. There is no statement/guarantee of backwards compatibility yet.

clemensv commented 6 years ago

What Doug said. We need a tag to refer to for implementations. And then a next one and a next one.

duglin commented 6 years ago

On the 4/19 call we all agreed that the v0.1 tag is there to provide the group a well defined version of the spec to which people can code to for the interop event at CNCF/Kube-con. It does not mean that anything in our docs is "set in stone" - everything is still changeable via future PRs. No statement of backwards compatibility is being made either.

Agreed to close this issue based on this agreement.