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

Move to Vert.x HTTP Client #2764

Closed
gastaldi opened this issue Jan 31, 2021 · 10 comments
Closed

Move to Vert.x HTTP Client #2764

gastaldi opened this issue Jan 31, 2021 · 10 comments

Comments

@gastaldi
Copy link
Contributor

Okhttp is no longer a single JAR and 4.x brings transitive dependencies (the kotlin-stdlib which is 1.5Mb).

There is a discussion about adopting Vert.x as the HTTP client here: quarkusio/quarkus#14662

Fixing this issue needs to also fix the problem reported in #2632

@jorsol
Copy link
Contributor

jorsol commented Mar 3, 2021

Ideally, (but this might bring its own challenges) is to be agnostic of the HTTP Client, and make it pluggable, if users want to use Apache HttpClient 5, or OkHttp 4, or Vert.x HTTP Client, or what I consider a nice zero-dependency: Java 11 HttpClient, would be great, this way Kubernetes-Client will not force a particular dependency.

And yes, this is probably a more invasive change and is subject to more challenges that just stick to one dependency.

@gastaldi
Copy link
Contributor Author

gastaldi commented Mar 3, 2021

@jorsol I think that's also possible, considering an abstraction layer is introduced in the HTTP operations part.

@stale
Copy link

stale bot commented Jun 1, 2021

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@shawkins
Copy link
Contributor

shawkins commented Mar 9, 2022

Julien Viet provided an outline of a client support at shawkins/httpclient#1

There is still a fair amount of work to be done.

@shawkins
Copy link
Contributor

shawkins commented May 6, 2022

@metacosm @vietj @manusa I've picked this up again at https://github.com/fabric8io/kubernetes-client/tree/vertx-client

There are plenty of unsupported things and todos left. I'll take another couple of passes over it to see if I can get the consume and interceptor logic roughed in - then reach out for anything that isn't easy to move forward.

It's not yet used, but later I'm guessing we'll need to use the abstract builder to support creating derived clients - the only really tricky thing there is the modification of the interceptors that we do for the openshift logic. It switches to specific token interceptor, or removes itself when making the refresh call.

@manusa
Copy link
Member

manusa commented May 7, 2022

@metacosm @vietj @manusa I've picked this up again at https://github.com/fabric8io/kubernetes-client/tree/vertx-client

Thx!

There are plenty of unsupported things and todos left. I'll take another couple of passes over it to see if I can get the consume and interceptor logic roughed in - then reach out for anything that isn't easy to move forward.

Oh, I was under the impression that shawkins/httpclient#1 has the main hiccups addressed and would be possible after #3971 😞

@shawkins
Copy link
Contributor

@vietj @metacosm I've made a few more changes, such as bringing in the WebClient dependency for interceptor handling. However it will speed things along if I can get some help with the following (marked with a TODO or unsupported exception in the code):

  • Confirmation that some of the config properties are not needed / meaningful - including should a general readTimeout be enforced
  • A few more config property implementations, such as SSL support, proxy support (seems like user / password may need to be passed separately),
  • Interceptor implementation - probably needs to be added per request to support the derived client modification of interceptors. Also needs to be applied to WebSocket requests.
  • Mapping to and from InputStream (send as a body, or receive) - is there another library for this already for compatibility with ReadStream/WriteStream?
  • Mapping to consumes methods

@shawkins
Copy link
Contributor

Updated the pr a little bit more: https://github.com/fabric8io/kubernetes-client/tree/vertx-client - updated to master and pulled in a couple of prospective changes, such as removing the need for a sendAsync implementation (which addresses the InputStream/Reader line item above - except for when it's used in a POST).

However it's still pretty far from functional. Wiring in interceptors before a websocket invocation is straight-forward. Afterwards looks problematic - I think the only thing we can do is add exception handling to the future and try to differentiate between a response issue, and a more general IO problem.

We also aren't able to use the AbstractXXX test classes in the vertx module due to the conflicting junit versions.

@manusa
Copy link
Member

manusa commented Dec 15, 2022

Work on the Vert.x HttpClient implementation is also being tracked at quarkusio/quarkus#29520

@manusa manusa mentioned this issue Jan 12, 2023
11 tasks
@manusa manusa added this to the 6.4.0 milestone Jan 19, 2023
@manusa manusa modified the milestones: 6.4.0, 6.4.1 Jan 31, 2023
@manusa
Copy link
Member

manusa commented Feb 6, 2023

Quarkus has finally migrated to use the Vert.x HttpClient implementation. Closing this issue as complete.

As we gather feedback from the Vert.x implementation, in the future, we should consider to make Vert.x the default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants