Skip to content

2472: Add support for container_name in docker-compose file #2741

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

Conversation

reggiemcdonald
Copy link

Closes: #2472

This PR adds support for container_name in the docker-compose file by pulling the container names out in ParsedDockerComposeFile and adding some additional logic in DockerComposeContainer

This is my first time contributing to the repo, so any feedback is welcomed.

@kiview kiview self-assigned this Jun 27, 2020
This commit updates the ParsedDockerComposeFile to pull out the container name for a service if included in the compose file. DockerComposeContainer was then updated with logic to use this specified name if present.
This commit adds tests for compose files with container_name specified. A compose file was added for this purpose.
@reggiemcdonald reggiemcdonald force-pushed the 2472-compose-container-name branch from aa38a96 to 4f7fab1 Compare July 2, 2020 04:04
@reggiemcdonald
Copy link
Author

reggiemcdonald commented Jul 2, 2020

I'm not really sure about this failing CI. Any suggestions? I just rebased with master, and it wasnt failing before the rebase

@bsideup
Copy link
Member

bsideup commented Jul 2, 2020

@reggiemcdonald I just triggered a re-run, there was some issue with downloading Gradle stuff

@reggiemcdonald
Copy link
Author

@bsideup thanks!

@wnyffenegger
Copy link

@rnorth @bsideup @kiview having this feature is a blocking issue for me is there a plan to merge this change in?

@stale
Copy link

stale bot commented Jan 24, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this.

@stale stale bot added the stale label Jan 24, 2021
@d3vAdv3ntur3s
Copy link

Thanks @reggiemcdonald great work

Is this still looking to be merged in? Currently the latest version (as of writing this) is 1.15.2 and this issue is still casuing problems.

@stale stale bot removed the stale label Mar 14, 2021
@james-toussaint
Copy link

Hi @reggiemcdonald, thank you! Do you have any plan on merging it @rnorth?

@kadaan
Copy link

kadaan commented May 5, 2021

@reggiemcdonald Can this get merged?

@eitzenbe
Copy link

Any update on landing this?

@matthieusb
Copy link

Hello. I had the same issue recently, I would be grateful if this PR could be merged. Thanks for the hard work !

@cunhap
Copy link

cunhap commented Nov 16, 2021

@rnorth @bsideup @kiview Can this PR be merged, please?

@svalexxx
Copy link

Is there any news about this PR?

@swanysimon
Copy link

@bsideup or @reggiemcdonald , what's the status here? If you don't have time or energy here, I'm happy to take over the PR

@jghoman
Copy link

jghoman commented Aug 16, 2022

Would also love to see this merged, and can take it over if someone needs to. Status?

@kiview
Copy link
Member

kiview commented Aug 24, 2022

Thanks for the PR @reggiemcdonald, but unfortunately we have to close it. Mainly, there are 2 reasons why we don't want to add support for container_name to DockerComposeContainer:

  1. It requires us to further parse the Docker Compose file and its format has been slightly volatile in the past. So we generally want to avoid further semantically parsing the Docker Compose file, independent from the actual container_name key.
  2. Using container_name would break existing DockerComposeContainer features, like scaling.
  3. As the Testcontainers library, we want to avoid the usage of fixed things, such as ports, names, etc., since those introduce the anti-pattern of integrated tests that can fail depending on the state of the environment in which they are running.

@orpheusx
Copy link

orpheusx commented Jan 3, 2025

@kiview The reasons for closing this seem fairly arbitrary from my point of view. Responding to your numbered reasons in order:

  1. Format volatility ebbs and flows over time but if the project wants to support compose files at all it seems better to at least not actively get in the way (by throwing exception over valid syntax.) My sense is that V2 has been more stable. (Note: the issue with container_name applies to the newer ComposeContainer as well as the now deprecated DockerComposeContainer.)
  2. Not sure how this would break scaling. An issue with Testcontainers Cloud?
  3. I agree that users should avoid environment state dependencies but providing supporting docker-compose files at all seems to be a pragmatic choice that recognizes the need that have been expressed above and in the original ticket, Support container_name in docker-compose file #2472.

For what it's worth, I just ran into this issue and would really like to see a solution because, in most other ways, Testcontainers is awesome.

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

Successfully merging this pull request may close these issues.

Support container_name in docker-compose file