-
Notifications
You must be signed in to change notification settings - Fork 26
Add support for additional data in the spec #63
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
Conversation
|
I would prefer if we called it "customContent" instead of "data". I believe that the intention is to provide a placeholder for any custom data that we in the spec call "content". This customContent could either be used for testing new content parameters in the protocol before proposing them to become standardized content parameters, or it could be used really as custom parameters for data that is not intended to be included in the procotol but rather remain specific to a certain use case. |
I have a preference for short and simple names, and I don't mind to convey the idea that CDEvents can be used as an envelop to transport extra data too. But
My intention is to provide a placeholder for any data that the sender would like to put in there.
Yes, definitely fields that are not yet in the spec could be transported via this field, until a standard name is specified for them. This is what I intend to do for a POC I'm working on, because not all required fields are in the spec yet. |
Add a new field. Organise the spec document so that it's clear which are the three root elements of a CDEvent and how the spec document itself is structured. Fixes: cdevents#60 Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
|
Fixed linter issues. |
I believe it is good to prefix the field with "custom", so that it is clear that the data put in there is not standardized by the protocol.
Well, I'm not sure about that "CDEvents channel". What would that mean? To me it is still a "CloudEvents channel". And I believe that, in accordance with our current structure, the parameters that identifies the subject for each event type lies on top level of the subject object, and any additional data is in the "contents" sub object in there. Therefore, I don't think it makes sense to add any custom data to the top level of the subject, but only to the contents sub object. Or? Would it still be a valid CDEvent that any consumer can understand if there are additional top level subject identity parameters? Anyway, I might miss some use case there, so adding the custom data on top level of the event would work for me as well. If we keep this custom data on the top level I'd prefer if we call it "customData". If we declare it under the subject, on same level as "content" I'd prefer if it was called "customContent". I don't really like the term "third-party" in this case. Third-party to whom? Probably not to the producer of the event. It is just that the service/system that the producer is executing or monitoring contains more info than what the spec declares, so from the producer's perspective it is rather "additional data" instead of "third-party data". |
Yeah, the proposal is to add it top level, since the data may not be specific to the subject only.
Yeah, |
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Co-authored-by: Emil Bäckmark <emil.backmark@ericsson.com>
Co-authored-by: Emil Bäckmark <emil.backmark@ericsson.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
e-backmark-ericsson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there :)
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
spec.md
Outdated
| - '{"mydata1": "myvalue1"}' | ||
| - 'VGhlIHZvY2FidWxhcnkgZGVmaW5lcyAqZXZlbnQgdHlwZXMqLCB3aGljaCBhcmUgbWFkZSBvZiAqc3ViamVjdHMqCg==' | ||
|
|
||
| #### customDataEncoding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realised that this should be "customDataContentType" instead.
The encoding (base64 or plain text) is decided based on the content-type and the binding format.
However the encoding alone does not give enough information to the consuming side to be able to parse the "customData".
I will rename this to "customDataContentType" and say that it is modelled after https://github.com/cloudevents/spec/blob/v1.0.2/cloudevents/spec.md#datacontenttype
I will need to add a section to the CloudEvent binding to describe that the "application/json" format will be rendered as json, and any other format will be base64 encoded.
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
spec.md
Outdated
|
|
||
| #### customData | ||
|
|
||
| - Type: [`String`][typesystem] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The customData value doesn't need to be string, as long as the content-type is application/json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good point, shall I use "object" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the type, since we don't really constraint the type of custom data, we only have requirements about the encoding depending on the content type
Co-authored-by: Emil Bäckmark <emil.backmark@ericsson.com>
Co-authored-by: Emil Bäckmark <emil.backmark@ericsson.com>
Co-authored-by: Emil Bäckmark <emil.backmark@ericsson.com>
Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
e-backmark-ericsson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy now :)
Add custom data to the SDK as described in cdevents/spec#63 Go objects can be passed directly, and the SDK will attempt to marshal them as json. Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Add custom data to the SDK as described in cdevents/spec#63 Go objects can be passed directly, and the SDK will attempt to marshal them as json. Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Add custom data to the SDK as described in cdevents/spec#63 Go objects can be passed directly, and the SDK will attempt to marshal them as json. Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Add custom data to the SDK as described in cdevents/spec#63 Go objects can be passed directly, and the SDK will attempt to marshal them as json. Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Fixes: #60
Signed-off-by: Andrea Frittoli andrea.frittoli@gmail.com