-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Make env_file optional #3955
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
Make env_file optional #3955
Conversation
d16d3f0 to
114563b
Compare
114563b to
e44d3a8
Compare
|
I have also updated the failing test and rebased with master. |
|
Hey, is this something we'd like to see in the codebase? Happy new year! |
|
Closing this one, since it didn't get much attention. Feel free to ping me to reopen it and rebase it if we believe it's a good fit 😃 |
|
This is useful, what it needs to be merged? |
|
Whoa whoa, would love to see this. |
|
What is preventing this from getting merged? |
|
👍 please add this! |
e44d3a8 to
108907f
Compare
108907f to
55d483c
Compare
|
Are there any new updates on this MR? |
This makes sure: * the actual file is not necessary, meaning that you don't have to create it each time you clone (given that usually `.env` files are ignored) * you still get a warning if the file doesn't exist, so you can be aware of times where a file should have been loaded but eventually didn't, for example due to a typo Signed-off-by: Antonis Kalipetis <akalipetis@gmail.com>
Signed-off-by: Antonis Kalipetis <akalipetis@gmail.com>
55d483c to
8f3403f
Compare
|
Agree, this would be a nice feature to have... in our use-case, Could be made backward compatible if necessary so that both of the following would work: Default/Required: env_file: docker-compose.envBackward/Forward Compatible: env_file:
path: docker-compose.env
required: falseThe above is similar to how the build: .But also accepts a nested dict: build:
context: .
args:
SOME_BUILD_ARG=1Etc... |
|
I would love to see this accepted. |
|
@akalipetis can this be merged? |
|
@dnephin |
|
This would fit my needs as implemented. I'm of the opinion that we break backward-compatibility here, perhaps incrementing the docker-compose version number. The new behavior is the one I believe most people will want, so we shouldn't require that more verbose syntax (nested I defer to the compose team's opinion though. I'm not sure what this behavior change would require for docker-compose format versioning. |
Despite the default being provided, if docker-compose.yml (by extension, docker/common.yml) specifies an `env_file`, the file is required to exist. Related issue: docker/compose#3560 Related PR: docker/compose#3955 For now, we must continue to create this file.
|
I would really like to have this as well! +1!!! Also: This is almost open for a year? Disappointed it's not merge earlier. Creating empty .env files just to satisfy the check is not acceptable. |
| try: | ||
| env.update(env_vars_from_file(env_file)) | ||
| except ConfigurationError as exc: | ||
| log.warn('{} - ignoring'.format(exc.msg)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though unlikely, a missing file isn't the only reason a ConfigurationError could be thrown. When the path exists but is not a file (e.g. a directory), a ConfigurationError will also be thrown. In that case, a blocking error seems more appropriate than a passive warning.
Instead of a try/except, perhaps we should check file existence directly.
if os.path.exists(env_file):
env.update(env_vars_from_file(env_file))
else:
log.warn('env_file {} does not exist - ignoring'.format(env_file))There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If @akalipetis makes this change, would this be merge-able?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: removed. sorry missed to read the thread.
|
Goal: Allow new contributors† to get set up by simply running Maintainers, do you agree that this is a worthy goal, in line with the project's philosophy? If so, we might also consider also:
The most significant consideration for this PR may be the backward-compatibility issue. These are the options I see:
Maintainers, you know the project needs best. Which option seems the most beneficial to you? We're willing to carry out the work but need a decision on the direction. Thanks! 😄 †to projects that use docker-compose |
|
👍 |
|
@KlaasH string interpolations would work, except for the cases when images use scripts that check if the variable is set rather than if it is empty. So, with As for using It is not possible to extend the YAML reference section in |
Like others have stated, running a bunch of Can we reconsider @ytilis's suggestion --^ from Nov 8, 2017 to add a |
|
One important aspect of this issue is that it already behaves this way to some extent. This loads .env file if it exists. If it does not exist it does nothing. This loads .env if exists, blows up if it does not... |
|
Currently there is no way to override environment variables with an optional environment variable file, unless it's explicitly created empty, or every environment variable is explicitly put in the @ytilis suggests adding an option for
(Note that Could we get agreement from the maintainers that this approach will be approved, so someone can create a merge request? @ulyssessouza: I'm tagging you since @shin-, @dnephin, and @aanand have all seem to quit the project entirely. @jlivermont, @anujbiyani: You two may also be interested in this reply. |
|
I have to agree with @tomasfejfar, this is inconsistent. Sometimes we have good reasons not to provide particular env files to particular users. Eg, an env file with development and production variables. I'm fine with the |
|
my ugly hack for now is to have an empty file in my repository (aptly named version: '3'
services:
test:
image: alpine
env_file:
- ${ENV_FILE:-.empty}using "no" env_file: $ grep . .empty .env
grep: .env: No such file or directory
$ docker-compose run test sh -c set | grep MYVAR
$using $ grep . .empty .env
.env:ENV_FILE=.env
.env:MYVAR0=foo
.env:MYVAR1=bar
$ docker-compose run test sh -c set | grep MYVAR
MYVAR0='foo'
MYVAR1='bar'
$using some other env_file: $ grep . .empty .env prod.env
.env:ENV_FILE=prod.env
prod.env:MYVAR0=hello
prod.env:MYVAR1=goodbye
$ docker-compose run test sh -c set | grep MYVAR
MYVAR0='hello'
MYVAR1='goodbye'it's hacky at best, but serves the purpose of having only a single simple file containing the environment for a container and which can be present or not (and be git-excluded) |
|
@ulyssessouza tagging again as this is a no-brainer in my eyes. If people are able to hack and workaround to achieve the same end then it might as well be explicitly supported. Doing it to protect consumers of these files only leads to put them at the mercy of workarounds. The people who want to setup their projects this way will find a way to do it supported or otherwise. |
|
If your file is named exactly Example: |
|
What's the status on this? I have a list of env_files where some of them are optional. Right now, docker-compose complains that they are missing and throws an error. Current workaround is to manually |
|
Agreed, I encountered this today as well and I did the same thing |
- Add .empty file, which is used, when there is no ENV_FILE set in .env - Kudos to umlaeute for the hack (docker/compose#3955)
|
I am working on a "monorepo" with a single |
|
@ulyssessouza Please take a look at this, and specifically @umlaeute's comment, this is possible with weird hacks already, so why not support it an official way, otherwise people are going to become dependent on methods not recommended by the official team that may end up breaking. |
|
Appreciate tomasbedrich's workaround to this problem but I wholeheartedly agree with leonaves and others, I don't understand why a workaround/hack which dirties either git or the compose config would be seen as a better solution than simply supporting this? |
|
In dev environment for each service our team wants to have: We want developer to:
We don't want developer to:
We could just push into repository empty It would be a life saver to have |
|
So far every single tool or service I've worked with simply assumed all env files to be optional by default. I mean, if you need to have them as required they should be some config files or required parameters. I've got extremely confused noticing that docker-compose has this custom behavior that puts hundreds of people into a dirty-hack area. |
Sorry for necrobumping, but I find the argument that this would make Compose files less portable is conjecture at best. In practice, the lack of an optional include path actually limits the portability of our project. Let me elaborate: At work we maintain a fairly large (I think) docker-compose.yml with about 10 services. This docker-compose.yml (and related .env files) is used for all environments along our DTAP flow from development all the way up to production. With a combination of extends and environment variable interpolation we've been able to make it so it can be easily configured to a certain environment. The development environment configuration works out of the box with sensible defaults. However, sometimes for certain external services alternate configuration need to be configured. Of course, this can be done by editing the original env files's or There is no elegant, fool-proof, convenient way to easily override environment values in files that are ignored in git by default. The only way to do this somewhat seamlessly is by using As you can see, I'm trying to make my compose setup as portable as possible for other developers, and the lack of an optional env file either requires me to limit my options, jump through strange hoops or, ultimately, resort to managing files outside of my docker-compose setup. That is the opposite of compatibility. |
|
I really wish this got merged. We often use |
This issue was closed with a comment that attempts to give a rationale, and that comment has an overwhelming response of thumbs down. The comment gives a rationale that is not relevant to the OP issue. The OP issue specifies that they have the file specified by The comment is completely ignorant to the OP specific requirement and issue, therefore the comment is not on topic and not a rational reason to close the issue. |
A "rationale" can not be based on feelings. This is an irrationale. Are you telling your users that their problems won't be resolved in this project and that we should use a different project? Which one do you recommend? Or are you asking us to fork this? This is confusing. |
|
I'm normally loathe to create new issues when clearly there's active discussion elsewhere, but given there's been no updates from maintainers that I see since 2017 I've opened a new issue. Hopefully that gets on a maintainer's radar with either suggestions on alternatives, what would be required to see something like this merged, or an updated "won't fix" response. |
|
Why not just add a new feature? services:
rails-app:
env_file:
- .env.development
optional_env_file:
- .env.development.local |
Bruh I aint sharing my env file of secret keys 😤 |
|
As a workaround, I'm using that syntax (pointing by default on my-service:
image: my-service:1.2.3
env_file:
- ${MY_SERVICE_ENV_FILE:-/dev/null}You can then either use: |
|
Anyone still coming here (like I did) for this feature, it is implemented now, as of Docker Compose See this comment by BuriedStPatrick:
|
This makes sure:
.envfiles are ignored)Fix #3560
Signed-off-by: Antonis Kalipetis akalipetis@gmail.com