-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Feat/update compose container #9871
base: main
Are you sure you want to change the base?
Conversation
…propriate docker image in the ComposeContainer.
…ion of docker and update the constructors
…vVarOrUserProperty checking if the value is not empty
3ae5baa
to
10d6485
Compare
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.
@@ -61,36 +62,74 @@ public class ComposeContainer extends FailureDetectingExternalResource implement | |||
|
|||
public static final String COMPOSE_EXECUTABLE = SystemUtils.IS_OS_WINDOWS ? "docker.exe" : "docker"; | |||
|
|||
private static final DockerImageName DEFAULT_IMAGE_NAME = DockerImageName.parse("docker:24.0.2"); | |||
private static final String DEFAULT_DOCKER_IMAGE = "docker:27.5.0"; |
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.
Do not override default image.
public static DockerImageName getDockerImageName() { | ||
return DockerImageName.parse( | ||
TestcontainersConfiguration | ||
.getInstance() | ||
.getEnvVarOrUserProperty("compose.container.image", DEFAULT_DOCKER_IMAGE) | ||
); |
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.
This is a deprecated way to do it and given we are offering a new constructor to set the image, this doesn't apply anymore.
45b2894
to
2dd3b3d
Compare
In the #9222 a user is specifying a property with the field name
compose.container.image
and theComposeContainer
when it creates thecomposeDelegate
variable is always picking the24.0.2
version of docker. The change that I am proposing is to use theTestcontainersConfiguration
in order to parse the propertycompose.container.image
if it exists otherwise continue as it was in the past. Updated the constructors based on the feedback.