Skip to content
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

Temporarily cut some attributes for 0.1? #163

Closed
inlined opened this issue Apr 15, 2018 · 11 comments
Closed

Temporarily cut some attributes for 0.1? #163

inlined opened this issue Apr 15, 2018 · 11 comments

Comments

@inlined
Copy link
Contributor

inlined commented Apr 15, 2018

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:

  • schemaURL and eventTypeVersion: there have been questions whether these are redundant.
  • contentType: it's not clear to me whether this is needed if every serialization would include it (e.g. HTTP's Content-Type header) or that mixed-encoding might not make sense (Is there value in an XML string as a JSON event data? Is there a difference between a JSON stringified object and a JSON object as a JSON event data?)
  • extensions: I think everybody agrees we need this in the spec, but we explicitly punt on discussing any until later milestones. Maybe we shouldn't have a pre-baked API until use cases are on the table.
@inlined
Copy link
Contributor Author

inlined commented Apr 15, 2018

@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
Copy link
Collaborator

duglin commented Apr 15, 2018

@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
Copy link
Contributor Author

inlined commented Apr 16, 2018

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
Copy link
Contributor Author

inlined commented Apr 16, 2018

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
Copy link
Contributor

contentType is referenced in #158, #157, #148, and #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" #138

@duglin
Copy link
Collaborator

duglin commented Apr 17, 2018

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
Copy link
Collaborator

duglin commented Apr 17, 2018

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

@inlined
Copy link
Contributor Author

inlined commented Apr 18, 2018

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
Copy link
Collaborator

duglin commented Apr 19, 2018

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
Copy link
Contributor

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

@duglin
Copy link
Collaborator

duglin commented Apr 19, 2018

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.

@duglin duglin closed this as completed Apr 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants