-
Notifications
You must be signed in to change notification settings - Fork 5.1k
CAMEL-22980 - fix "specify custom port" in infra for Zookeeper #21388
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
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🤖 CI automation will test this PR automatically. 🐫 Apache Camel Committers, please review the following items:
|
|
The custom port is needed by the |
| : -1; | ||
| if (imageName == null) { | ||
| return new ZooKeeperContainer(ZooKeeperContainer.CONTAINER_NAME, clientPort); | ||
| return new ZooKeeperContainer(); |
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.
are the changes in ZooKeeperLocalContainerInfraService really needed?
aren't the ones in ZooKeeperContainer enough?
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.
I don't know. My goal was to get back to a more stable main branch as soon as possible. So found the commit introducing the issue and tried to revert. We can adjust the previous work later on so that the main branch (and all other developments) is not impacted
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.
ok, the changes to ZooKeeperContainer are ok, but the changes to ZooKeeperLocalContainerInfraService should be reverted, and, in order to support both camel test, and camel infra run with custom or default port scenarios, the following diff can be applied.
|
51c492e to
20118a5
Compare
|
failing check is unrelated to this PR. i tis due to a regression on main branch, see this PR #21402 to revert this regression |
|
@apupier In the diff there is a change to ZooKeeperLocalContainerInfraService could you apply this change too? |
|
Ah can we get the last bit attempted, and then also rebase on main |
Signed-off-by: Aurélien Pupier <[email protected]>
20118a5 to
2c77a47
Compare
Description
Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.