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

Use existing environment variables in .env values #3

Closed
wants to merge 1 commit into from

Conversation

adamtorres
Copy link

I needed to use some existing environment variables when creating the .env for certain systems. As is, dotenv does nothing but strip the outside quotes from the values. All this commit adds is calling os.path.expandvars() to replace variables with their values.

@jpadilla
Copy link
Owner

@adamtorres not sure if this could introduce undesired behavior for others, might consider introducing it with an additional option like read_dotenv(expand_variables=True). Thoughts?

@jamieconnolly
Copy link
Contributor

@adamtorres @jpadilla — I'm definitely 👍 on this, and think this should be the default behaviour.

The use case I can see is one where a user doesn't want the value replaced. In that situation, most production systems default is to replace the value, unless it's escaped. This would mean, they would have to have a different type of value between development (not escaped) and production (escaped). Would this not potentially cause further confusion when the user hits production?

@jpadilla
Copy link
Owner

@jamieconnolly hmm I think I understand what you mean. So you're saying we should instead try to mimic whatever "production systems" do by default, which is replace the value instead of it being escaped?

@jamieconnolly
Copy link
Contributor

@jpadilla — Yes. It just seems like a more seamless approach to me.

@jpadilla
Copy link
Owner

It'd be interesting to see if this behavior works like you'd expect with this PR. I have no clue how to do any other way though. If anyone wants to give it a try, they're more than welcome.

@jamieconnolly
Copy link
Contributor

With this method, it only replaces variables that already exist in the environment, so the following use-case wouldn't behave as expected:

FOO=test
BAR=$FOO

Also, expandvars leaves the variable untouched if it doesn't exist in the environment, whereas the default behaviour is to replace it with "".

BAR=$FOO

# shell output
BAR=""

# expandvars output
BAR='$FOO'

I hope all of that makes sense. I've put together a PR containing these changes in #7.

@adamtorres
Copy link
Author

@jamieconnolly Your pull request, #7, looks much more complete than this one. I put this together mainly to answer a problem I had and hadn't considered much beyond that limited scope.

Plus, you've got tests. Tests are good.

@jpadilla
Copy link
Owner

jpadilla commented Dec 6, 2014

Superseded by #7. Will publish in a few minutes to PyPI. Thank you all!

@jpadilla jpadilla closed this Dec 6, 2014
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.

3 participants