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

Cannot customize Executor on WebSocketClient #10547

Closed
joakime opened this issue Sep 19, 2023 · 1 comment · Fixed by #10548 or #10589
Closed

Cannot customize Executor on WebSocketClient #10547

joakime opened this issue Sep 19, 2023 · 1 comment · Fixed by #10548 or #10589
Assignees
Labels
Bug For general bugs on Jetty side Sponsored This issue affects a user with a commercial support agreement
Milestone

Comments

@joakime
Copy link
Contributor

joakime commented Sep 19, 2023

Jetty version(s)
10.0.16

Jetty Environment
n/a

Java version/vendor (use: java -version)
All

OS type/version
All

Description
If a developer wants to customize the Executor of the WebSocketClient, it is not possible, as the internal final WebSocketComponents is initialized with a new Executor from scratch, not using the one from the HttpClient (or other common / shared locations)

@joakime joakime added the Bug For general bugs on Jetty side label Sep 19, 2023
joakime added a commit that referenced this issue Sep 19, 2023
@joakime joakime moved this to 🏗 In progress in Jetty 12.0.2 FROZEN Sep 19, 2023
@joakime joakime added this to the 10.0.x milestone Sep 19, 2023
@joakime joakime self-assigned this Sep 19, 2023
@lorban lorban added the Sponsored This issue affects a user with a commercial support agreement label Sep 19, 2023
joakime added a commit that referenced this issue Sep 19, 2023
Signed-off-by: Joakim Erdfelt <[email protected]>
@joakime joakime moved this to 🏗 In progress in Jetty 10.0.17 / 11.0.17 FROZEN Sep 19, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 10.0.17 / 11.0.17 FROZEN Sep 22, 2023
joakime added a commit that referenced this issue Sep 22, 2023
… HttpClient (#10548)

* Issue #10547 - Allow Executor of WebSocketClient to be customized via HttpClient

Signed-off-by: Joakim Erdfelt <[email protected]>
@sbordet sbordet reopened this Sep 25, 2023
@sbordet
Copy link
Contributor

sbordet commented Sep 25, 2023

The fix for this issue in #10548 is not optimal, because we are now exposing explicitly 2 ways of passing executors, which is more confusing than before.
Furthermore, it break JPMS.

We should hide the fact that WebSocketComponents is necessary, also because it is a WebSocket core class that should not be necessary when creating instance of the upper-layer WebSocket APIs.
WebSocketComponents is only for implementation use.

@sbordet sbordet moved this from ✅ Done to 🏗 In progress in Jetty 10.0.17 / 11.0.17 FROZEN Sep 25, 2023
sbordet added a commit that referenced this issue Sep 27, 2023
The HttpClient and WebSocketComponents will now try to share as many components as possible.

Signed-off-by: Simone Bordet <[email protected]>
@joakime joakime moved this to 🏗 In progress in Jetty 12.0.2 FROZEN Sep 27, 2023
sbordet added a commit that referenced this issue Sep 27, 2023
Added tests after review.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Sep 28, 2023
The HttpClient and WebSocketComponents will now try to share as many components as possible.

Signed-off-by: Simone Bordet <[email protected]>
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 10.0.17 / 11.0.17 FROZEN Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side Sponsored This issue affects a user with a commercial support agreement
Projects
None yet
3 participants