Extend 'addFixedExposedPort' method with InternetProtocol argument#586
Extend 'addFixedExposedPort' method with InternetProtocol argument#586bsideup merged 3 commits intotestcontainers:masterfrom
Conversation
| * @param protocol an internet protocol (tcp or udp) | ||
| * @return this container | ||
| */ | ||
| public SELF withFixedExposedPort(int hostPort, int containerPort, InternetProtocol protocol) { |
There was a problem hiding this comment.
I'd rather not leak docker-java implementation details into the public API. Instead I think we should use our own Enums here.
| */ | ||
| public enum InternetProtocol { | ||
|
|
||
| tcp, |
There was a problem hiding this comment.
Can you change the enums to uppercase please? 🙂
There was a problem hiding this comment.
@kiview If it's not your code style conventions I prefer to leave it as is rather then do implicit 'toLowerCase' and 'toUpperCase' modifications every time. Want to recall that this part is case sensitive in Docker and docker-java.
There was a problem hiding this comment.
we use uppercase for enum values. If you want, you can create toDockerNotation/fromDockerNotation methods to encapsulate the conversion
There was a problem hiding this comment.
@bsideup Thank you for explanation. Added toDockerNotation/fromDockerNotation methods.
…style conventions
|
See also #554 - this doesn't need to be done as part of this PR, but is useful to be aware of. It looks like this PR (e.g. creation of the enum) will help - thanks. |
| */ | ||
| protected void addFixedExposedPort(int hostPort, int containerPort) { | ||
| portBindings.add(String.format("%d:%d", hostPort, containerPort)); | ||
| addFixedExposedPort(hostPort, containerPort, InternetProtocol.TCP); |
There was a problem hiding this comment.
AFAIR if the protocol is not specified, it will use both, no? If so, this is a breaking change
There was a problem hiding this comment.
No @bsideup by default it is TCP. Proof: https://docs.docker.com/config/containers/container-networking/
|
@agafox thanks for your contribution! 💪 |
|
Why is this methord protected in 1.19.0 protected void addFixedExposedPort(int hostPort, int containerPort, InternetProtocol protocol) { |
5 participants
No description provided.