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 reaper for compose #465

Closed
wants to merge 1 commit into from
Closed

Conversation

oGi4i
Copy link

@oGi4i oGi4i commented Jun 15, 2022

Closes #372

@oGi4i oGi4i marked this pull request as ready for review June 15, 2022 13:26
@oGi4i
Copy link
Author

oGi4i commented Jun 15, 2022

@mdelapenya
Not the most elegant solution, would love to hear your thoughts on this.

@mdelapenya
Copy link
Collaborator

Hi @oGi4i, thanks for your first contribution to TC-go.

We are currently reviewing #463, which could cause this PR to break.

If you agree, I'd like to have your PR after the other one.

@oGi4i
Copy link
Author

oGi4i commented Jun 15, 2022

If you agree, I'd like to have your PR after the other one.

Sure, no problem

@mdelapenya mdelapenya added the compose Docker Compose. label Oct 6, 2022
@mdelapenya
Copy link
Collaborator

Hi @oGi4i we have merged #476, which adds native support for docker compose. Would you mind revisiting this PR and update to that code? I can assist if needed, or we can even ping @baez90 for that.

Thanks in advance

@mdelapenya
Copy link
Collaborator

@oGi4i I'm closing this issue as stale.

OTOH it would be ideal if the compose APIs allow passing labels to the container CreateOptions (they only provide it for the RunOptions type), which would way simplify this implementation, simplify passing reaper labels as we do in https://github.com/testcontainers/testcontainers-go/blob/main/docker.go#L996-L999

Probably worth it to ping the Docker folks for that support. If they are not responsive, we could try designing this implementation in a similar way that you started here, but adapted to the new code.

In any case, thanks for your time here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compose Docker Compose.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Containers created by Docker Compose are not cleaned by Ryuk
2 participants