-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Replace Vert.x in CruiseControlAPI with native HttpClient API from Java #10791
Conversation
f4db806
to
1616376
Compare
6a608e2
to
933a36b
Compare
.../main/java/io/strimzi/operator/cluster/operator/assembly/KafkaRebalanceAssemblyOperator.java
Outdated
Show resolved
Hide resolved
.../main/java/io/strimzi/operator/cluster/operator/assembly/KafkaRebalanceAssemblyOperator.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Gantigmaa Selenge <[email protected]>
Signed-off-by: Gantigmaa Selenge <[email protected]>
975ab51
to
81911d5
Compare
/azp run regression |
Azure Pipelines successfully started running 1 pipeline(s). |
...n/java/io/strimzi/operator/cluster/operator/resource/cruisecontrol/CruiseControlApiImpl.java
Outdated
Show resolved
Hide resolved
...ava/io/strimzi/operator/cluster/operator/resource/cruisecontrol/CruiseControlClientTest.java
Show resolved
Hide resolved
Thanks @katheris for reviewing the PR. I have now added the missing join() for one of the tests and removed the unused variable. |
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.
Thanks, LGTM
Signed-off-by: Gantigmaa Selenge <[email protected]>
b301fe4
to
5bb738b
Compare
/azp run regression |
Azure Pipelines successfully started running 1 pipeline(s). |
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 a nit but overall LGTM. Thanks!
...n/java/io/strimzi/operator/cluster/operator/resource/cruisecontrol/CruiseControlApiImpl.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Gantigmaa Selenge <[email protected]>
Going to merge this. I don't think it needs a new regression running because latest commit doesn't have any big impact it was just a nit that @tinaselenge addressed based on my comment. |
Type of change
Select the type of your PR
Description
Replace use of Vertx HTTP and Future from CruiseControl API implementation with Java CompletableFuture and HTTP API.
Checklist
Please go through this checklist and make sure all applicable tasks have been done