compose-go icon indicating copy to clipboard operation
compose-go copied to clipboard

add services circular dependency check when parse config file

Open junyuanz1 opened this issue 2 years ago • 2 comments

fix a bug reported in https://github.com/docker/compose/issues/9526 by adding service dependency check after reading the Docker compose configuration YAML file.

Signed-off-by: Junyuan Zheng [email protected]

junyuanz1 avatar Jun 07 '22 06:06 junyuanz1

Hey @ndeloof, let me know what you think about this change. I understand there will be some duplicate code between this repo and the go/compose repo, but I still think circular dependency check should live in the repo since valid YAML configuration file is the responsibility of the compose-go repo.

I am pretty new to those Docker repositories, so please let me know if you want me to move this change to the compose repo.

To solve the duplicate code issue, I am planning to to remove the HasCycle check from the go/compose once this change is checked in.

junyuanz1 avatar Jun 07 '22 06:06 junyuanz1

Hi @junyuan-z, thanks for the contribution, I appreciate you taking the time to look at this!

While your approach looks good on its face, I still think there's room to discuss where and what kind of dependency checking of the project there should be at load time. The spec doesn't really have anything to say about this, and the current behavior does allow cycles in the project definition; depending on what options you specify, there may or may not be an error when starting services. For an example, consider a compose file with two services foo and bar that depend on each other, what should be the behavior if we run docker compose up --no-deps foo? Currently, that definition is allowed, and compose will only start foo in that case.

nicksieger avatar Jun 09 '22 20:06 nicksieger