-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add a getter for Toxiproxy exposed port #2513
Conversation
This is a good idea, but I just want to have a think about naming - I think that |
Hi @rnorth, I agree, the name could be improved. What do you think about |
@rnorth soft ping :) |
Sorry - naming this is hard! I think personally I'd prefer I think we should probably skip this for today's release - sorry that my delay in getting back slowed things down. |
Thanks, |
e9641f0
to
53b4606
Compare
Before this change, there only existed a getter for the mapped port which is useful for accessing the container from the host. A new getter can be useful when a Toxiproxy is setup between two different Docker containers to simulate network instabilities.
Increase Jedis timeout from the default 2 seconds to 10 seconds. This timeout value is used for both socket connect timeout and socket read timeout. It looks like there is a delay between creating a proxy and it being operational/accessible.
53b4606
to
dd31f5b
Compare
Hi @rnorth, this PR is now updated:
Please re-review, thanks in advance :) |
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 @lutovich this is great. I just have a couple of tiny tweaks to suggest, then I think we're good to go 🙂
modules/toxiproxy/src/main/java/org/testcontainers/containers/ToxiproxyContainer.java
Outdated
Show resolved
Hide resolved
1805d1c
to
b335f8f
Compare
@rnorth thanks a lot! I committed all suggestions and squashed them into one commit |
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.
Looks great - thanks for the contribution @lutovich!
Hello!
I used Toxiproxy container to simulate a connection cut between two different Docker containers. It was a bit hard to determine what exposed port of the proxy corresponds to the exposed port of the target container. The code I ended up with did
toxiproxy.getExposedPorts().get(1)
which is not very intuitive becausetoxiproxy.getExposedPorts().get(0)
is the proxy control port. Thus this PR. Hope it makes sense :)Before this change, there only existed a getter for the mapped port which is useful for accessing the container from the host. A new getter can be useful when a Toxiproxy is setup between two different Docker containers to simulate network instabilities.