fix regression when compose name is specified #500
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lionello
force-pushed
the
lio-fix-name-override
branch
from
0ccab28 to
dcd5e6c
Compare
edwardrf
reviewed
Jun 26, 2024
| @@ -96,7 +97,7 @@ func getDefaultProjectOptions(workingDir string, extraOpts ...cli.ProjectOptions | |||
| // get compose file path set by COMPOSE_FILE | |||
| cli.WithConfigFileEnv, | |||
| // if none was selected, get default compose.yaml file from current dir or parent folder | |||
| cli.WithDefaultConfigPath, | |||
| // cli.WithDefaultConfigPath, NO: this ends up picking the "first" when more than one file is found | |||
There was a problem hiding this comment.
Here is the link:
https://github.com/compose-spec/compose-go/blob/9d0d133e13d0955e27520c6317d08822b7c5de5f/cli/options.go#L165
Should we keep it consistent though?
6 tasks
jordanstephens
added a commit
that referenced
this pull request
Aug 6, 2024
Fixes #509 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. - [x] Add support for multiple compose files with `-f`. - [x] Modify `compose.LoadCompose` to leverage the `compose-go` library to find default compose file paths if one isn't passed explicitly. - [x] 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. - [x] Remove `toomany` test case since there is nothing meaningful to test anymore. - [x] 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. - [x] lazily verify compose path, to avoid requiring it for commands like `defang version` which do not use it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
also adds a new
toomanytest case to test the case when there are more than 1 compose files in a folder