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

Support for multiple compose files #603

Merged

Conversation

jordanstephens
Copy link
Member

@jordanstephens jordanstephens commented Aug 1, 2024

Fixes #509 and #369

In #500, we were concerned that the presence of both a compose.yaml and docker-compose.yaml file could be confusing, so we decided to return an error and exit early. However, compose-go does log out a warning in this scenario, as does docker compose. We've changed our mind and decided to aim for closer parity with docker here by reverting to logging the warning and continuing with compose.yaml.

While implementing this, in an effort to bring us to closer parity with docker compose, we decided to go ahead and add support for passing -f multiple times—adding support for multiple compose files.

  • Add support for multiple compose files with -f.
  • Modify compose.LoadCompose to leverage the compose-go library to find default compose file paths if one isn't passed explicitly.
  • Avoid returning an error if multiple default compose files are found. compose-go will always pick compose.yaml over docker-compose.yaml. It will also log a warning message explaining this to stderr.
  • Remove toomany test case since there is nothing meaningful to test anymore.
  • Add a simple test at the command level for something like defang version to make sure it doesn't try to load a compose file during intialization. In time, we should add command-level tests for everything with a mock server.
  • lazily verify compose path, to avoid requiring it for commands like defang version which do not use it.

@jordanstephens jordanstephens marked this pull request as ready for review August 2, 2024 18:37
@jordanstephens jordanstephens force-pushed the jordan/simplify-handling-of-multiple-compose-files branch from 2258b20 to 4b1032f Compare August 2, 2024 18:42
src/pkg/types/error.go Outdated Show resolved Hide resolved
@jordanstephens jordanstephens force-pushed the jordan/simplify-handling-of-multiple-compose-files branch 2 times, most recently from 6ad28c1 to ae05864 Compare August 2, 2024 18:48
src/cmd/cli/command/commands_test.go Outdated Show resolved Hide resolved
src/pkg/cli/compose/loader.go Outdated Show resolved Hide resolved
src/pkg/cli/compose/compose_test.go Outdated Show resolved Hide resolved
@jordanstephens jordanstephens force-pushed the jordan/simplify-handling-of-multiple-compose-files branch from b6aebf5 to f1f0a89 Compare August 6, 2024 18:47
Use the compose-go library to find the default path if no path is
passed instead of replicating their logic in our codebase.

Once a path is obtained, we can derive the current working
directory.

If we are using default paths, we will test if there are
potentially conflicting default compose files: compose.yaml _and_
docker-compose.yaml.
This error is no longer used
@jordanstephens jordanstephens force-pushed the jordan/simplify-handling-of-multiple-compose-files branch from f1f0a89 to 1200054 Compare August 6, 2024 18:54
@jordanstephens jordanstephens changed the title Warn instead of fail when a folder contains multiple compose files Add support for multiple compose files Aug 6, 2024
@jordanstephens jordanstephens changed the title Add support for multiple compose files Support for multiple compose files Aug 6, 2024
@jordanstephens jordanstephens force-pushed the jordan/simplify-handling-of-multiple-compose-files branch from 1200054 to 93c37e2 Compare August 6, 2024 19:54
src/pkg/cli/compose/loader.go Outdated Show resolved Hide resolved
@jordanstephens jordanstephens force-pushed the jordan/simplify-handling-of-multiple-compose-files branch from 02978c2 to 4e18bd5 Compare August 6, 2024 22:10
@jordanstephens jordanstephens force-pushed the jordan/simplify-handling-of-multiple-compose-files branch from 4e18bd5 to 6341688 Compare August 6, 2024 22:15
@jordanstephens jordanstephens merged commit 97c70c1 into main Aug 6, 2024
7 checks passed
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.

Handle folder with multiple compose files
3 participants