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

RESTEASY-3006 Allow microprofile-client to inject Vertx engine for HTTP/2 communication #7

Merged
merged 4 commits into from Oct 20, 2021

Conversation

liweinan
Copy link
Contributor

@liweinan liweinan commented Sep 10, 2021

resolves #8

@liweinan liweinan changed the title Try to use Vert.x Engine RESTEASY-3006 Allow microprofile-client to inject Vertx engine for HTTP/2 communication Sep 11, 2021
Copy link
Contributor

@jamezp jamezp left a comment

Choose a reason for hiding this comment

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

Overall I think this looks okay. I want to think about if we can get it to work without having to use the RestClientBuilderImpl directly. One thing that comes to mind is doing something like:

HttpClientOptions options = new HttpClientOptions();
options.setSsl(true);
options.setProtocolVersion(HttpVersion.HTTP_2);
options.setUseAlpn(true);

builder.setProperty("org.jboss.resteasy.http.client.engine", new VertxClientHttpEngine(Vertx.vertx(), options));

Then the RestClientBuilderImpl itself looks for the property. It requires knowledge of the ClientHttpEngine, but that seems okay as that is a public API and IMO the RestClientBuilderImpl should not be a public API.

testsuite/integration-tests/pom.xml Outdated Show resolved Hide resolved
@liweinan
Copy link
Contributor Author

@jamezp Unfortunately RestClientBuilder doesn't have a setProperty() method and it's defined by eclipse community:

image

Shall I submit a request to add a method into the interface to the eclipse community? Is there any workflow to submit it?

@jamezp
Copy link
Contributor

jamezp commented Sep 22, 2021

@jamezp Unfortunately RestClientBuilder doesn't have a setProperty() method and it's defined by eclipse community:

image

Shall I submit a request to add a method into the interface to the eclipse community? Is there any workflow to submit it?

There should be as it extends javax.ws.rs.core.Configurable. I had a typo in my example and it should be builder.property("org.jboss.resteasy.http.client.engine", new VertxClientHttpEngine(Vertx.vertx(), options));. The builder.getConfiguration().getProperty("org.jboss.resteasy.http.client.engine") could be used in the implementation to lookup the requested engine.

@liweinan
Copy link
Contributor Author

@jamezp Thanks for the suggestion! I'll go on working on this :D

@liweinan
Copy link
Contributor Author

@jamezp I have modified all the places you've mentioned, could you please help to do a final check? Thanks! :D

(the test error in above I checked is because of Github CI platform network problem)

After this PR is merged I'll:

  • Write a blog related with HTTP/2 client support on resteasy side and resteasy-microprofile side.
  • Create a similar PR to resteasy-reactive-client (to Quarkus).

@liweinan liweinan merged commit a229efd into resteasy:main Oct 20, 2021
@liweinan
Copy link
Contributor Author

@jamezp I merged the PR, if there is any problem pls let me know.

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

Successfully merging this pull request may close these issues.

Allow microprofile-client to inject Vertx engine for HTTP/2 communication
2 participants