Skip to content
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: Add container builder member to depend on other Docker resources #832

Merged
merged 20 commits into from
Mar 24, 2023

Conversation

HofmeisterAn
Copy link
Collaborator

@HofmeisterAn HofmeisterAn commented Mar 13, 2023

What does this PR do?

This pull request adds the member DependsOn(IContainer) to the container builder, allowing it to depend on other containers. Internally, we will create and start the dependent containers (resources) first. The same applies for networks and volumes. Please note that we do not have an overloaded member for network and volume. Instead, we use the equivalent member WithNetwork(INetwork) and WithVolumeMount(Ivolume, string).

Why is it important?

This makes it easier for developers to configure test environments and configurations that depend on other resources. We can also use it internally, for example, the WebDiver container configuration will be much simpler.

Related issues

Follow-ups

  • Remove (dispose) dependent resources. This is not super urgent, because the Resource Reaper will take care of it anyway.

@HofmeisterAn HofmeisterAn added the enhancement New feature or request label Mar 13, 2023
@netlify
Copy link

netlify bot commented Mar 13, 2023

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit e8570cc
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-dotnet/deploys/641d6fa143764c0008d4316a
😎 Deploy Preview https://deploy-preview-832--testcontainers-dotnet.netlify.app/api/create_docker_container
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.


public Task DisposeAsync()
{
return _container.DisposeAsync().AsTask();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cristianrgreco @eddumelendez @mdelapenya @kiview Currently, dependent resources are not being removed when the parent container is disposed, as these resources might be shared across different configurations. Ryuk is responsible for cleaning them up.

My idea is to maintain an internal reference counter that increments with every create call. When the reference counter equals 1, we can remove the dependent resources as well (while disposing the parent). WDYT?

A general review would be appreciated. PR contains minor general improvements..

@HofmeisterAn HofmeisterAn marked this pull request as ready for review March 22, 2023 15:23
@HofmeisterAn HofmeisterAn changed the title Feature/826 add depends on icontainer feat: Add container builder member to depend on other Docker resources Mar 24, 2023
@HofmeisterAn HofmeisterAn merged commit f587f1c into develop Mar 24, 2023
@HofmeisterAn HofmeisterAn deleted the feature/826-add-depends-on-icontainer branch March 24, 2023 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement]: Support depending on other Testcontainers (Docker) resources
1 participant