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

make extensions follow the same pattern as normal attrs #505

Merged
merged 3 commits into from
Sep 19, 2019

Conversation

duglin
Copy link
Collaborator

@duglin duglin commented Sep 12, 2019

Fixes #498

This PR makes CE extensions follow the same rules as all other CE attributes. Meaning:
1 - they MUST be serialized with ce- prefixes in transports like binary-http
2 - they MAY have a secondary serialization, but they MUST still have the ce- prefixed one too.
3 - if they do, and the values differ, then the ce- version is the CE attribute. The other one, if picked-up by the middleware, is 'some other' metadata that it's passing along.

This means that a CE receiver no longer needs to look any place except in one location for ALL CloudEvents data. This also means that ALL CloudEvents extensions will be picked up by ALL receivers. W/o this PR, it's possible for someone to define a CE extension that maps to an existing HTTP header (like datacontenttype does) and because the HTTP header it maps to is a well-known HTTP header, the CE receiver might not realize it's a CE extension and not pass it along to the app. This PR avoids that problem.

In the end, this PR gets us back to our "first principles" that CloudEvents is trying to standardize the location of a well-defined set of metadata - even if it ends up duplicating the data. For example, we've talked about how some of our data would be duplicated from the "business logic"(data) and that's ok. So, let's do the same for extensions too. By allowing some attributes (CE defined or extensions) to ONLY have a different serialization (and possibly location), we're violating our own stated goal. But, allowing for a "secondary" serialization allows for us to have a single location for all CE data but then also allow for existing metadata defined by other specs to still be properly filled-in.

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

@Vlaaaaaaad
Copy link

I very much like the idea of limiting the scope to ce- only.

I'll let the transport experts review this tho.

Signed-off-by: Doug Davis <dug@us.ibm.com>
@duglin duglin changed the title [WIP] make extensions the same [WIP] make extensions follow the same pattern as normal attrs Sep 14, 2019
@duglin
Copy link
Collaborator Author

duglin commented Sep 14, 2019

ok - I updated all of the docs that I think need to be updated.
Please take a look. If there are alternatives then please open a complete PR before EOD Tuesday so we can consider it. This is the last open v1.0 issue/PR and we'd like to get to v1.0-rc1 on next week's call!!

@duglin
Copy link
Collaborator Author

duglin commented Sep 14, 2019

@clemensv
Copy link
Contributor

clemensv commented Sep 16, 2019

This is good.

Why are you asking for the datacontenttype info to flow double for MQTT5, though? That protocol version should work just like HTTP.

@duglin duglin changed the title [WIP] make extensions follow the same pattern as normal attrs make extensions follow the same pattern as normal attrs Sep 17, 2019
@clemensv
Copy link
Contributor

LGTM

Copy link
Contributor

@deissnerk deissnerk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found a typo
LGTM

spec.md Outdated
@@ -419,13 +421,31 @@ extensions that might be of interest.
Each specification that defines how to serialize a CloudEvent will define how
extension attributes will appear.

Extensions attribtue MUST be serialized using the same general pattern as all
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Extensions attribtue/Extension attributes

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks - fixed

Signed-off-by: Doug Davis <dug@us.ibm.com>
Signed-off-by: Doug Davis <dug@us.ibm.com>
@duglin duglin added the v1.0 label Sep 19, 2019
@duglin
Copy link
Collaborator Author

duglin commented Sep 19, 2019

Approved on the 9/19 call.

@duglin duglin merged commit f41061e into cloudevents:master Sep 19, 2019
@duglin duglin deleted the issue498 branch September 19, 2019 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTP Binary spec: possibility of dropping attributes
4 participants