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

Jetty HTTP client integration with WebClient [SPR-15092] #19658

Closed
spring-projects-issues opened this issue Jan 4, 2017 · 11 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jan 4, 2017

Brian Clozel opened SPR-15092 and commented

Both implementations could take WebSocketClient implementations as examples of how to deal with backpressure with each library. The configuration model should also copy the current arrangement in WebSocketClient implementations, since each library deals with client instances and configuration objects in a different way.


Issue Links:

Referenced from: pull request #1783, and commits spring-projects/spring-boot@f74dd7d

0 votes, 7 watchers

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Looks like there is now a Jetty ReactiveStreams HTTP Client. Perhaps we can try this for 5.0 still?

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

No commit for 5 month on jetty-reactive-httpclient, so given the early stage of that project I am not sure we should work on this for 5.1 ... Any thoughts Rossen Stoyanchev?

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Considering it exposes a Reactive Streams API, it should be relatively low effort (and risk) from our end. Certainly worth a quick experiment I think. Good point about the lack of activity. We should least indicate our intention (Jetty mailing list perhaps?) and ask whether they still plan to keep this effort up if we start filing issues and providing feedback.

@spring-projects-issues
Copy link
Collaborator Author

Akram commented

is there any update on this? If not then I suggest to revisit the documentation to specify that WebClient only supports Netty rightnow

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

I'm scheduling for 5.1 RC1 since we do intend to give it a try at least.

@spring-projects-issues
Copy link
Collaborator Author

Simone Bordet commented

@Rossen, lack of activity is due to the fact that it's a wrapper around Jetty's HttpClient which is stable.
We are happy to improve the reactive wrapper if it needs to, and we definitely want to maintain it.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

(y)

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

I have a draft implementation with green tests for WebClientIntegrationTests and SseIntegrationTests, except WebClientIntegrationTests#shouldSendLargeTextFile with is failing. I have raised jetty-project/jetty-reactive-httpclient#3 with a repro project on Jetty side to ask for a fix.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

Latest status update:

  • The issue about WebClientIntegrationTests#shouldSendLargeTextFile has been fixed
  • Jetty RS HTTP client now implements RS TCK tests for its Processor and Publisher implementations!
  • I have polished implementation based on Simone feedback
  • The main TODO is on buffer interrop, it works with buffer copy but fails with Mono when using ByteBuffer wrapping. It may be due to the deferred chunk release due to DataBufferUtils#join. I am currently discussing with Simone about that.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

Pull request submitted, please send me your feedback.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

Note that Jetty RS Client supports HTTP/2.

We should be able to add HTTP/2 tests at some point since Jetty RS client supports h2c and OkHttp MockServer 3.11 is going to support it as well. Another solution would be to make these tests only active when running Java9+, but that's not ideal especially with parameterized tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants