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

Dotenv overrides existing env variables #5

Closed
jsantos17 opened this issue Jul 28, 2015 · 13 comments
Closed

Dotenv overrides existing env variables #5

jsantos17 opened this issue Jul 28, 2015 · 13 comments

Comments

@jsantos17
Copy link

Common behavior of dotenv packages is not overriding existing environment variables. .env by default. env should not write over existing variables or at least provide an option to avoid overriding:

Examples of the common behavior
Go: https://github.com/joho/godotenv/blob/master/godotenv_test.go#L122
Ruby: https://github.com/bkeepers/dotenv/blob/master/spec/dotenv/environment_spec.rb#L25
Haskell: https://github.com/stackbuilders/dotenv-hs/blob/master/spec/Configuration/DotenvSpec.hs#L21
Clojure: https://github.com/tpope/lein-dotenv/blob/master/test/lein_dotenv/plugin_test.clj#L11

Why does this implementation work differently?

@theskumar
Copy link
Owner

@jsantos17 thanks for reporting this.

Why does this implementation work differently?

I think it's primarily because it was un-noticed and is an undocumented behaviour. I'm happy to make a new release, with the new behaviour in place. Want to submit a PR?

The new behaviour is arguable to have it or not have it. But I would suggest to go with what's happening elsewhere and be consistent with libraries.

@theskumar
Copy link
Owner

There should also be an configuration to toggle either of the behaviour while loading the .env file, to maintain backward compatibility.

E.g. https://github.com/stackbuilders/dotenv-hs#configuration

@jsantos17
Copy link
Author

@theskumar I think I can work on a PR over the weekend. Thanks for the quick response 👍

@gsong
Copy link
Contributor

gsong commented Sep 20, 2015

@jsantos17 The current behavior does not override existing entries in os.environ: https://github.com/theskumar/python-dotenv/blob/master/dotenv.py#L21

@theskumar
Copy link
Owner

@gsong I believe, what @jsantos17 meant is that if .env file is present and then same key/value is present in both as system environment variable and also in .env file.

@gsong
Copy link
Contributor

gsong commented Sep 22, 2015

@theskumar In that case, the system environment variable takes precedence, according to the code and my observed behavior. I think that's the correct behavior, with potentially a feature enhancement to override as necessary, right?

@theskumar
Copy link
Owner

@gsong the key/value defined in the dotfiles takes precedence as these values are loaded later and also because you are explicitly putting those values in .env file. So the default is whatever written in .env takes precedence. That way you are able to have multiple .env on a single machine.

PS: I never required to have a conflicting variable during my development/production. But I'm not sure what kind of use cases people have around it.

@gsong
Copy link
Contributor

gsong commented Sep 22, 2015

@theskumar I don't think that's the behavior though. Since the code explicitly reads os.environ.setdefault(k, v). That means if k already exists, then it's not overridden by load_dotenv, right?

Or am I misunderstanding this thread?

@theskumar
Copy link
Owner

@gsong I guess, I have to say you are right!!! Thanks for clarifying my miss-understandings! I'll writeup some test cases and we should be good!

@Pomax
Copy link

Pomax commented Aug 9, 2016

Going to have to be contrarian here: a good 12-factor application relies on defaults, which can be overwritten by system environment values, which in turn can be overwritten by explicit env file values, so that devs have several levels of control, and have a clear means to override settings when necessary: you only use an env file during development, and instead make sure your deployment setup, which is private and secured, has the correct environment variables set.

As such, it makes far more sense to have dotenv load in the system evironment variables, and then override each that is explicitly different in the env file, with a unified access: rather than os.environ.get you now use

from dotenv import load_dotenv, find_dotenv
env = load_dotenv(find_dotenv())
someval = env.get("someval", "default value")

to get either the default value, or the system override if there is one, or the env override on either if a developer needing an explicitly different value. The reason being that env files are trivial to change, whereas setting environment variables is super hard, especially cross-platform (good luck temporarily setting some env vars in Windows, for instance)

@maxkoryukov
Copy link
Contributor

@milonimrod
Copy link
Contributor

Hi,
@theskumar Any reason why this can't be an option with default behavior to not override and given the option to override?

@theskumar
Copy link
Owner

Support for overriding system variable is available in 0.7.0 now. Thanks Nimrod :)

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

No branches or pull requests

6 participants