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

datacontentencoding is not OPTIONAL #481

Closed
cneijenhuis opened this issue Aug 9, 2019 · 9 comments
Closed

datacontentencoding is not OPTIONAL #481

cneijenhuis opened this issue Aug 9, 2019 · 9 comments

Comments

@cneijenhuis
Copy link
Contributor

The notational convention states:

For clarity, when a feature is marked as "OPTIONAL" this means that it is OPTIONAL for the Producer [...] of a message to support that feature. In other words, a producer can choose to include that feature in a message if it wants [...]

The datacontentencoding is listed as an OPTIONAL attribute, however producers MUST or MUST NOT set the attribute - in other words, they MUST support the feature (which can mean they have to omit the attribute):

The attribute MUST be set if the data attribute contains string-encoded binary data. Otherwise the attribute MUST NOT be set.

The same will be true for datacontenttype if we adopt either #470 or #471 .

My suggestion is to either move them to REQUIRED attributes and clarify that this includes a requirement to NOT set the attribute, or to create a new section for attributes that are producers are REQUIRED to use correctly (incl. omitting them), but consumers have to handle them not being there.

@rperelma
Copy link
Contributor

rperelma commented Aug 9, 2019

Good point @cneijenhuis I agree with your suggestion. I think # 1 is best, and explain more clearly what REQUIRED means.

@duglin
Copy link
Collaborator

duglin commented Aug 10, 2019

@jroper @clemensv do you think you could modify the PRs to add text in the "constraints" section to indicate when they are REQUIRED?

@cneijenhuis
Copy link
Contributor Author

@duglin If I'm not mistaken, their PRs already indicate when it is REQUIRED in the constraints. My point is that an OPTIONAL attribute MUST NOT state it is REQUIRED in some circumstances in the constraints section.

A "skimming" reader may skip the optional attributes - it would be quite a surprise that these are not optional, after all...

@duglin
Copy link
Collaborator

duglin commented Aug 14, 2019

@cneijenhuis if #484 is adopted, does the new header for the "optional attributes" section address your concern?

@duglin duglin added the v1.0 label Aug 14, 2019
@cneijenhuis
Copy link
Contributor Author

Mostly, but I think the Notational Convention section should be changed as well, e.g. to:

For clarity, when a feature is marked as "OPTIONAL" this means that it is
OPTIONAL for both the [Producer](#producer) and [Consumer](#consumer) of a
message to support that feature. In other words, a producer can choose to
include that feature in a message if it wants, and a consumer can choose to
support that feature if it wants. A consumer that does not support that feature
will then silently ignore that part of the message. The producer needs to be
prepared for the situation where a consumer ignores that feature. An
[Intermediary](#intermediary) SHOULD forward OPTIONAL attributes.

When a feature is marked "conditional", this means the [Producer](#producer)
MUST support the feature, which CAN mean that the producer MUST NOT set an
attribute. An [Intermediary](#intermediary) MUST forward conditional attributes.

@duglin
Copy link
Collaborator

duglin commented Aug 15, 2019

I'm not sure that kind of text is required since to me the text around the MUSTs makes it clear that if the condition specified is met, then the MUST applies. The classic example is:

if present, MUST be a non-empty string

The presence of this requirement in the "optional attributes" section doesn't really change whether or not people are required to adhere to the MUST. Basically the word "OPTIONAL" in the header of the section is meaningless other than to say these attribute might appear while the ones in the REQUIRED section will appear. Perhaps we should find another word other than OPTIONAL for the title. Would that help? Or just title the section "Conditional Attributes" ??

But that's just me :-) If people think more text is needed, then we just need to find the right wording because not everything in that section is "conditional". Perhaps something like this at the top of the "OPTIONAL and conditional attributes" section:


While the attributes listed in this section are not mandatory for all CloudEvents, some of them might be based the particular use case.


wdyt?

@duglin
Copy link
Collaborator

duglin commented Aug 19, 2019

@cneijenhuis what's the status of this one? Want to open a PR?

@cneijenhuis
Copy link
Contributor Author

#492 may resolve this - either it gets adopted, or I'll do a PR.

@duglin
Copy link
Collaborator

duglin commented Sep 5, 2019

Closed due to #492 - if incorrect please let me know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants