-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add support for specifying Docker image in DockerComposeContainer and ComposeContainer #9871
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 support for specifying Docker image in DockerComposeContainer and ComposeContainer #9871
Conversation
3ae5baa to
10d6485
Compare
eddumelendez
left a comment
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 for your contribution. I've left some comments. Also adding proper docs and tests would be nice.
core/src/main/java/org/testcontainers/containers/ComposeContainer.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/testcontainers/containers/ComposeContainer.java
Outdated
Show resolved
Hide resolved
45b2894 to
2dd3b3d
Compare
core/src/test/java/org/testcontainers/containers/ComposeContainerTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/testcontainers/containers/DockerComposeContainer.java
Outdated
Show resolved
Hide resolved
1e80f8d to
6c23e41
Compare
|
Is there any chance this could be fixed soon? It's a simple change, but it has a significant impact — currently, it's blocking our use of the Compose Module in CI. I've incorporated the review comments in this update; hopefully, this helps move things forward for a quick release. |
6c23e41 to
2660009
Compare
|
I have updated it with some tests it is failing on a random test on |
|
Any news on this? |
|
Hi our team is also looking to use this feature. |
|
Hi @eddumelendez can you review this and maybe merge it :) ? |
|
This is also blocking my company to use Testcontainers, would be amazing if this could be merged |
13f1052 to
5442e1b
Compare
|
still no review from the team ? |
…propriate docker image in the ComposeContainer.
…ion of docker and update the constructors
…vVarOrUserProperty checking if the value is not empty
aaf378b to
9df92b2
Compare
c60d8a3 to
14be0bb
Compare
14be0bb to
ee13d7d
Compare
|
Thanks for your contribution! |
| * | ||
| * @return this instance, for chaining | ||
| */ | ||
| public ComposeContainer withLocalCompose(boolean localCompose) { |
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.
Without this setter, localCompose is always true and could be inlined.
In the #9222 a user is specifying a property with the field name
compose.container.imageand theComposeContainerwhen it creates thecomposeDelegatevariable is always picking the24.0.2version of docker. The change that I am proposing is to use theTestcontainersConfigurationin order to parse the propertycompose.container.imageif it exists otherwise continue as it was in the past. Updated the constructors based on the feedback.