-
Notifications
You must be signed in to change notification settings - Fork 219
fix: remove okhttp dependency #1793
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
Conversation
0480f6b
to
9918f6f
Compare
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.
Just mad one comment. Also not sure if the main pom.xml should be reformatted, but otherwise LGTM
operator-framework/pom.xml
Outdated
|
||
<dependency> | ||
<groupId>io.fabric8</groupId> | ||
<artifactId>kubernetes-httpclient-jdk</artifactId> |
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.
Should not be this the vertx client?
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.
As mentioned previously off-line, I would like to have tests with different clients to see if there is any impact
@@ -54,6 +54,11 @@ | |||
<type>test-jar</type> | |||
<scope>test</scope> | |||
</dependency> | |||
<dependency> | |||
<groupId>io.fabric8</groupId> | |||
<artifactId>kubernetes-httpclient-okhttp</artifactId> |
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.
Why is okhttp in this module? Probably should be vertx
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.
Let's test with all the different clients in integration tests in a different PR. In this PR, I wanted to have "some" coverage of the different http client implementations without getting into all possible combinations.
@@ -38,6 +38,10 @@ | |||
<groupId>io.javaoperatorsdk</groupId> | |||
<artifactId>operator-framework</artifactId> | |||
</dependency> | |||
<dependency> | |||
<groupId>io.fabric8</groupId> | |||
<artifactId>kubernetes-httpclient-okhttp</artifactId> |
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.
Should not be just okhttp in one of these?
Also document how to use a different implementation instead of the okhttp one which is the default one and currently, the one we recommend to use. Vert.X implementation is also coming along nicely. JDK implementation should work if you're using Java 17 and up, and will work with 11 when some bug fixes land in 11.
Kudos, SonarCloud Quality Gate passed! |
Also document how to use a different implementation instead of the okhttp one which is the default one and currently, the one we recommend to use. Vert.X implementation is also coming along nicely. JDK implementation should work if you're using Java 17 and up, and will work with 11 when some bug fixes land in 11.
Also document how to use a different implementation instead of the okhttp one which is the default one and currently, the one we recommend to use. Vert.X implementation is also coming along nicely. JDK implementation should work if you're using Java 17 and up, and will work with 11 when some bug fixes land in 11.
Also document how to use a different implementation instead of the okhttp one which is the default one and currently, the one we recommend to use. Vert.X implementation is also coming along nicely. JDK implementation should work if you're using Java 17 and up, and will work with 11 when some bug fixes land in 11.
Also document how to use a different implementation instead of the okhttp one which is the default one and currently, the one we recommend to use. Vert.X implementation is also coming along nicely. JDK implementation should work if you're using Java 17 and up, and will work with 11 when some bug fixes land in 11.
Also document how to use a different implementation instead of the okhttp one which is the default one and currently, the one we recommend to use. Vert.X implementation is also coming along nicely. JDK implementation should work if you're using Java 17 and up, and will work with 11 when some bug fixes land in 11.
No description provided.