bindings icon indicating copy to clipboard operation
bindings copied to clipboard

feat: support `sse` protocol

Open jfallows opened this issue 1 year ago • 10 comments

Description

  • Added sse message binding object
  • Uses http protocol in server definition

Note: we are in the process of testing these proposed changes with zilla open source project to confirm they are suffcient to support sse message validation, etc.

See https://github.com/aklivity/zilla/issues/952

jfallows avatar Jun 07 '24 19:06 jfallows

Shouldn't this fall under the HTTP binding instead?

fmvilas avatar Jun 08 '24 09:06 fmvilas

Would users reading the bindings, know that the SSE-specific information is part of the HTTP bindings? Or is it more for convenience in maintaining them? 🤔

I mean leaning more towards keeping them separate I think 🤔

jonaslagoni avatar Jun 08 '24 14:06 jonaslagoni

Hey guys, thanks for the feedback.

SSE does have a standard content type for http responses, defining the SSE protocol framing, but the message payload defined by the application is contained in that framing, i.e. spanning one or more data fields per SSE event.

Therefore message schema types for validation need to be described at a higher level than the SSE http response, as SSE has its own logical channel of application defined messages.

jfallows avatar Jun 08 '24 14:06 jfallows

as SSE has its own logical channel of application defined messages

🤔 This wasn't obvious to me at first glance. Can't we map AsyncAPI channels to these logical channel definitions? Cause I have the feeling that's how it should be but speaking from pure gut feeling.

fmvilas avatar Jun 17 '24 17:06 fmvilas

as SSE has its own logical channel of application defined messages

🤔 This wasn't obvious to me at first glance. Can't we map AsyncAPI channels to these logical channel definitions? Cause I have the feeling that's how it should be but speaking from pure gut feeling.

Right, the endpoint is identified via URL such as https://example.com/server/sent/events.

Here we propose to identify the logical channel of messages via the request path, such as /server/sent/events above, much like the http and websockets bindings.

jfallows avatar Jun 17 '24 18:06 jfallows

Then I think http://example.com should be a server definition and /server/sent/events should be the address of the channel. So better to keep it out of the sse binding. What do you think?

fmvilas avatar Jun 20 '24 08:06 fmvilas

Then I think http://example.com should be a server definition and /server/sent/events should be the address of the channel. So better to keep it out of the sse binding. What do you think?

https://example.com/server/sent/events is the endpoint URL from the clients point of view, such as via JavaScript in the browser.

var events = new EventSource("https://example.com/server/sent/events");

Are you suggesting that the server definition would use http protocol instead of sse protocol?

If so, then we would have just the Message Binding Object, indicating the sse event type such as message to specify allowed sse message types?

jfallows avatar Jun 20 '24 22:06 jfallows

Yeah, exactly. That would simplify things IMHO. The server protocol could be http or http+sse. Either way should be fine.

fmvilas avatar Jun 20 '24 22:06 fmvilas

Yeah, exactly. That would simplify things IMHO. The server protocol could be http or http+sse. Either way should be fine.

@fmvilas I've gone with http for now, since there are more dimensions to this with secure and http or https seems simpler.

Please let me know if having an sse message binding object without specifying the protocol as sse at server level introduces any downstream issues for validation or parsers for code generation.

jfallows avatar Jun 20 '24 23:06 jfallows

Yeah now there's nothing in the AsyncAPI document saying that this API MUST use SSE. It only defines that "when using SSE", this message's event type is X. IMHO, there should be something else indicating that an API uses SSE, like server protocol http+sse or https+sse. I'd not allow sse as the server protocol because it lacks information about the HTTP counterpart, for instance, if it's secure (https) or not.

fmvilas avatar Jun 21 '24 06:06 fmvilas

This pull request has been automatically marked as stale because it has not had recent activity :sleeping:

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

There can be many reasons why some specific pull request 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 pull request forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

github-actions[bot] avatar Oct 20 '24 00:10 github-actions[bot]