-
Notifications
You must be signed in to change notification settings - Fork 981
HTTPCLIENT-2396 - add Server-Sent Events client to HttpClient5. #719
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
base: master
Are you sure you want to change the base?
Conversation
|
@arturobernalg Would it be OK for you to move implementation classes to |
69c4c69 to
33862f4
Compare
EventSource API + SseExecutor, CHAR/BYTE parsers, pluggable BackoffStrategy, Last-Event-ID and Retry-After.
79502c4 to
6eb32a6
Compare
|
@arturobernalg What is your motivation for proposing this new module? Do you have an application that would be using it? Is there any chance of someone else contributing to it as well? That would be the second module where you would be the sole maintainer. |
My motivation is to provide minimal, low-level support for Server-Sent Events on top of the existing async HttpClient APIs. Right now users who need SSE either implement their own ad-hoc parsing and reconnect logic, or pull in a separate client library. I do have concrete use cases for this in long-lived monitoring / event streams and would use this module in those applications instead of maintaining custom SSE code. The module is small, self-contained and builds only on existing HttpClient primitives. The public API is intentionally limited, so the maintenance cost should stay low even if I remain the primary maintainer. |
I don't recommend doing this. Package-private (default) visibility is the only reliable Java language mechanism we have to encapsulate implementation details. If we put implementation details in an |
@arturobernalg There were instances when a complete module had been contributed to the project in the past: caching in client and reactive in core, but in both instances those contributions were backed by a commercial entity. At the very least those modules had a user base. In your case there is no user base, so it is unclear how this new solution would be applied in real life settings. If there is no documentation no one will even find out about it unless they look at the source code. I have no objections to inclusion of this module into the project but I think your priorities needs to shift from producing more experimental features to documenting / applying those you have already contributed. |
@rschmitt That seems to imply that public API classes would have some implementation code in them that needed to be kept private / not exposed to the API consumers, because here is nothing stopping classes in impl from re-using package private code within that package. I understand this is more of a matter of taste, so let's agree to disagree here and let @arturobernalg decide what approach to take. |
| * a factory such as {@code SseExecutor#open(...)}), which wires the | ||
| * {@link EventSourceListener} that receives events.</p> | ||
| * | ||
| * <h3>Thread-safety</h3> |
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.
@arturobernalg Could you please also add @Contract annotations as well?
To clarify, my concern with an |
New SSE module for HttpClient5 with
EventSource,SseExecutor, and spec-compliant parsing.Pluggable reconnect strategies, resumable IDs, and Retry-After hints; H1/H2 ready.
Defaults are simple; advanced tuning exposed via builders and strategies.
Includes docs, examples, and tests; API surface ready for review.