Skip to content

Conversation

@ehsavoie
Copy link
Contributor

Motivation:

I needed to be able to process SSE events on the client side.

Conformance:

You should have signed the Eclipse Contributor Agreement as explained in https://github.com/eclipse/vert.x/blob/master/CONTRIBUTING.md
Please also make sure you adhere to the code style guidelines: https://github.com/vert-x3/wiki/wiki/Vert.x-code-style-guidelines

Copy link
Contributor

@vietj vietj left a comment

Choose a reason for hiding this comment

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

we need for now

  • at least 1 integration test with WebClient
  • SseBodyCodec and tests should be in vertx-web-common package instead where BodyCodec is

@ehsavoie ehsavoie force-pushed the sse_client branch 2 times, most recently from 4efdb3b to fdd4b43 Compare September 26, 2025 12:46
Copy link
Contributor

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

Thank you @ehsavoie !

How about moving SseEvent to the common module so that we could, later on, think about sse for the server without breaking things here?

cc @vietj

@ehsavoie
Copy link
Contributor Author

@tsegismont maybe I could make the SseEvent toString to acutally create a 'valid'SSE string with the new lines

@ehsavoie ehsavoie force-pushed the sse_client branch 2 times, most recently from b232c03 to fddd3ac Compare October 1, 2025 11:10
Copy link
Contributor

@vietj vietj left a comment

Choose a reason for hiding this comment

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

A few changes

Copy link
Contributor

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

Thank you @ehsavoie ! This looks great already. I think we need to make the new objects Vert.x API objects (annotated with @VertxGen or @DataObject).

Otherwise, they won't be available in generated bindings.

That might require a bit of refactoring, you can inspire from other codecs, like BodyCodec.

@ehsavoie ehsavoie force-pushed the sse_client branch 6 times, most recently from 9274840 to 5164652 Compare October 17, 2025 08:42
Copy link
Contributor

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

Thank you for the updates @ehsavoie !

Copy link
Contributor

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

Thank you @ehsavoie !

@tsegismont
Copy link
Contributor

@vietj @ehsavoie do we intend to backport this to 4.x ? I believe we should take this into account while implementing the data object (regarding requirements of data objects like empty/copy/json constructors)

Copy link
Contributor

@tsegismont tsegismont left a comment

Choose a reason for hiding this comment

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

Any reason for not moving the test from web-common to web-client?

Signed-off-by: Emmanuel Hugonnet <[email protected]>
@ehsavoie
Copy link
Contributor Author

@tsegismont I've move the test

@ehsavoie ehsavoie requested a review from tsegismont October 24, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants