Localstack support for HOSTNAME_EXTERNAL #1891
Conversation
rnorth
force-pushed
the
localstack-hostname-external
branch
from
135b1f6 to
cb65299
Compare
rnorth
changed the title
Co-authored-by: CauchyPeano <[email protected]>
rnorth
force-pushed
the
localstack-hostname-external
branch
from
cb65299 to
4272996
Compare
| [*.java] | ||
| indent_style = space | ||
| indent_size = 4 | ||
| # Never use star imports |
There was a problem hiding this comment.
Added this (IntelliJ-specific) configuration after I accidentally reset the import formatting style on GenericContainer. After this, we should converge towards consistent import style.
| @@ -523,6 +523,7 @@ private void applyConfiguration(CreateContainerCmd createCommand) { | |||
| } | |||
|
|
|||
| String[] envArray = env.entrySet().stream() | |||
| .filter(it -> it.getValue() != null) | |||
There was a problem hiding this comment.
Tangential change - noticed that we don't null check the environment variables we pass through, which seems likely to be a bug.
rnorth
changed the title
...s/localstack/src/main/java/org/testcontainers/containers/localstack/LocalStackContainer.java
Outdated
Show resolved
Hide resolved
| public class LocalstackContainerTest { | ||
|
|
||
| // without_network { | ||
| @ClassRule |
There was a problem hiding this comment.
Can we please at least use nested test classes if we use the rules?
Starting multiple localstack containers at once may be very slow in some CIs
There was a problem hiding this comment.
Shouldn't we even consider making this a plain test without rules?
Also considering our way forward, which would put rules into another module.
There was a problem hiding this comment.
I would very much be in favour of it :)
There was a problem hiding this comment.
Very good point about the weight of these containers.
TBH I think I prefer the nested test class approach here; it's not a situation where try-with-resources is particularly elegant, and because these tests are excerpted for docs I'd like to keep the container creation style consistent.
I'll push a change with nested classes in a moment.
…/localstack/LocalStackContainer.java Co-Authored-By: Sergei Egorov <[email protected]>
Replacement for #1422
It looks like I can't push commits to @CauchyPeano's branch, so creating a new PR with his work and my changes.