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

Message Payload required or not #928

Closed
stereomon opened this issue Apr 19, 2023 · 12 comments
Closed

Message Payload required or not #928

stereomon opened this issue Apr 19, 2023 · 12 comments
Labels
❔ Question A question about the spec or processes

Comments

@stereomon
Copy link

I already asked this question in the Slack Channel for the Specification and was asked to clarify it with an issue here.

https://www.asyncapi.com/docs/reference/specification/v2.2.0#definitionsMessage (The statement is the same for all higher versions)

It states that a message MUST have a payload. But what if I have a command message with the headers containing all the data I need? Do I really need to duplicate the data in the payload?

@derberg mentioned that it is confusing that on the message definition level, the spec says MUST and in the https://www.asyncapi.com/docs/reference/specification/v2.2.0#messageObject it is not marked as REQUIRED.

From my perspective, it shouldn't be a MUST and it shouldn't be REQUIRED to have a payload defined in each message specifically not when the headers contain all relevant data for me.

Is the documentation wrong or do I miss something?

@stereomon stereomon added the ❔ Question A question about the spec or processes label Apr 19, 2023
@github-actions
Copy link

Welcome to AsyncAPI. Thanks a lot for reporting your first issue. Please check out our contributors guide and the instructions about a basic recommended setup useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@derberg
Copy link
Member

derberg commented Apr 26, 2023

For me it is just an editorial error.
Whatever is in definition
Screenshot 2023-04-26 at 14 36 44

Is overwritten by Message Object definition that do not REQUIRE payload
Screenshot 2023-04-26 at 14 38 59

Also schema do not require it either: https://github.com/asyncapi/spec-json-schemas/blob/master/schemas/2.6.0.json#L894


My proposal is to say MAY in case of message payload in the definition section

@char0n @fmvilas @smoya @dalelane thoughts?

@dalelane
Copy link
Collaborator

Would the absence of a message payload definition imply "these messages have no payload"? Or simply that the payload is undefined?

If this ambiguity is unhelpful, then there might be value in keeping payload as required, but having a way of explicitly specifying it can be empty.

@stereomon
Copy link
Author

Would the absence of a message payload definition imply "these messages have no payload"?
Yes, messages having all needed data specified in the header for processing doesn't need a payload.

Making it a required field doesn't make sense from my point of view. Not sure how e.g. Kafka would react to a non present payload.

I would prefer to have this as an optional field in the specification.

@dalelane
Copy link
Collaborator

Sorry, I didn't explain myself clearly enough.

I wasn't questioning whether different messaging/streaming technologies tolerate the absence of a payload.

I was considering and discussing the best way to document and describe a message without a payload. Specifically, I was comparing one approach (that omitting the payload section from the doc specifies that there is no payload) with another approach (having a payload section in the AsyncAPI doc, and in that section you can explicitly say that the payload is empty)

Copy link
Member

derberg commented May 24, 2023

we are having the same situation as with headers, if headers are not provided, does it mean they are unknown or that there are no headers?

@dalelane
Copy link
Collaborator

That's true... my assumption would be that if something isn't specified, then it's unknown - that the most likely reason is that the author didn't think to specify it. And that's certainly my assumption with something like headers which are often not documented.

I'm not sure I'm strongly arguing for this, more thinking out loud... I was considering something along the lines of authors needing to explicitly specify something like null to specify that messages actually have no payload. But I think you're right that if we did that, then the same could/should apply to headers, too.

@fmvilas
Copy link
Member

fmvilas commented May 30, 2023

We should clarify but I think the absence of headers should mean they're "unknown" or "not defined". Probably because the author doesn't care about them too. There's a mechanism to define headers and payloads as empty. It should be something like the following:

type: object
additionalProperties: false

or

type: 'null'

However, I'd be careful with the null value because some systems may allow you to send a null value, which is not the same as "empty".

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Sep 28, 2023
@char0n
Copy link
Collaborator

char0n commented Oct 31, 2023

I would suggest to make an editorial change to replace the MUST with MAY word to remove ambiguity from the specification and to make things right.

The source of truth for me (tooling author) are actual objects definitions (Message Object, Channel Object) which supersedes everything else within the specification text. Maybe it would be worth mentioning this inside the spec itself? We're already dealt with the source of truth before in #781.

Question about the actual clarity is probably for another issue. For me personally omitting the payload means that my Message Object MAY have payload but I didn't care describing it. Given it's not possible to describe an absence of a value by the JSON Schema, the clarity is implicit but clear (correct me if I'm wrong).

@smoya smoya removed the stale label Oct 31, 2023
@smoya
Copy link
Member

smoya commented Oct 31, 2023

I would suggest to make an editorial change to replace the MUST with MAY word to remove ambiguity from the specification and to make things right.

I think we all agree on that by reading the rest of comments. I do, as well. Is anyone willing to create a PR with such a change? @stereomon perhaps?

We can move forward on that matter as a first and solid approach. Additional alternatives such as the one commented as well by @fmvilas here can keep evolving apart in this discussion IMHO.

@char0n
Copy link
Collaborator

char0n commented Oct 31, 2023

@smoya editorial change issued as #983

@smoya smoya closed this as completed Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❔ Question A question about the spec or processes
Projects
None yet
Development

No branches or pull requests

6 participants