Skip to content

Container definition API - #7714

Merged
eddumelendez merged 35 commits into
mainfrom
new-container-def-api
Oct 31, 2023
Merged

eddumelendez merged 35 commits into
mainfrom
new-container-def-api

Conversation

@eddumelendez

Copy link
Copy Markdown
Member

Introduce ContainerDef and apply it to PortForwardingContainer
and MongoDBContainer.

This reverts commit 7ccb450.
This reverts commit 402be60.
@eddumelendez

Copy link
Copy Markdown
Member Author

We can fix #7708 by switching to this.containerDef.addExposedTcpPorts(ports); here WDYT?

Comment thread modules/mongodb/src/main/java/org/testcontainers/containers/MongoDBContainer.java Outdated
Comment thread modules/mongodb/src/main/java/org/testcontainers/containers/MongoDBContainer.java Outdated
Comment thread core/src/test/java/org/testcontainers/containers/ExposedHostTest.java Outdated
@eddumelendez eddumelendez added this to the next milestone Oct 30, 2023
private boolean privilegedMode;

@NonNull
private List<VolumesFrom> volumesFroms = new ArrayList<>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we move it in a follow-up PR to avoid breaking changes?

Comment thread modules/neo4j/src/test/java/org/testcontainers/containers/Neo4jContainerTest.java Outdated
Comment thread core/src/test/java/org/testcontainers/containers/ExposedHostTest.java Outdated
Comment on lines -908 to -914
Map<String, String> combinedLabels = new HashMap<>();
combinedLabels.putAll(labels);
if (createCommand.getLabels() != null) {
combinedLabels.putAll(createCommand.getLabels());
}

createCommand.withLabels(combinedLabels);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change, as previously we were combining the labels

Comment on lines -908 to -914
Map<String, String> combinedLabels = new HashMap<>();
combinedLabels.putAll(labels);
if (createCommand.getLabels() != null) {
combinedLabels.putAll(createCommand.getLabels());
}

createCommand.withLabels(combinedLabels);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the order with regards to CreateContainerCmdModifier

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a change to to an undocumented internal behavior, we are fine with accepting it.

@eddumelendez
eddumelendez merged commit 1dba8d1 into main Oct 31, 2023
@eddumelendez
eddumelendez deleted the new-container-def-api branch October 31, 2023 16:39
@nitesh0420

Copy link
Copy Markdown

Sadly it broke our tests. 😄

@eddumelendez

Copy link
Copy Markdown
Member Author

Sorry, but that doesn't provide much information about the issue you are having. We have been reported about some regressions and some of them are fixed now, see this list. There is one more that should be fixed tomorrow. If you can not find yours, please feel free to open a new issue.

@illabefat

Copy link
Copy Markdown

Hi @eddumelendez,
There's a bug that waitingStrategy sometimes gets ignored: #8578.

fokion pushed a commit to fokion/testcontainers-java that referenced this pull request Jan 18, 2025
Introduce `ContainerDef` and apply it to `PortForwardingContainer`
and `MongoDBContainer`.

`ContainerDef` is an internal API used by `GenericContainer` to decouple
the state of container creation and the running state.

---------

Co-authored-by: Sergei Egorov <bsideup@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants