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

Override / interpolate of variables working incorrectly #256

Closed
hectorvs opened this issue Jun 18, 2020 · 5 comments
Closed

Override / interpolate of variables working incorrectly #256

hectorvs opened this issue Jun 18, 2020 · 5 comments
Assignees

Comments

@hectorvs
Copy link

@hectorvs hectorvs commented Jun 18, 2020

Bug is a combination of override and interpolate.

contents of files is as follows:

foo.txt

Name=foo
Interpolate="${Name}/baz"

bar.txt

Name=bar
Interpolate="${Name}/baz"

test file:

from dotenv import load_dotenv
import os

def print_env():
    for key in os.environ.keys():
       print("{}: {}".format(key, os.getenv(key)))

load_dotenv('foo.txt', override=True)
print_env()

load_dotenv('bar.txt', override=True)
print_env()

when second print_env() runs, the values are:

Name=bar
Interpolate=foo/baz # should be bar/baz

the reason for this is that the resolve_nested_variables method looks into existing ENV vars first and then looks at the current file's loaded values. (in our case, Name was loaded in the earlier file with value foo).

I believe the correct functionality would be to first look into the loaded file's values and then on the environment in order to keep the same behavior as the first file loaded.

@elbehery95
Copy link
Contributor

@elbehery95 elbehery95 commented Jun 23, 2020

Totally agree. This is annoying and will always enforce that the current env defined variable will still be substituted in the .env even if its defined at .env

This line should just be reversed

ret = os.getenv(name, new_values.get(name, default))

https://github.com/theskumar/python-dotenv/blame/e92ab1f18087c2ca058203f36535c43a0f824d8a/src/dotenv/main.py#L223

@bbc2 your thoughts and feedback are really appreciated

@bbc2
Copy link
Collaborator

@bbc2 bbc2 commented Jun 24, 2020

This indeed looks like a problem. Thanks for reporting, I'll look into it.

@bbc2 bbc2 self-assigned this Jun 24, 2020
@elbehery95
Copy link
Contributor

@elbehery95 elbehery95 commented Jun 24, 2020

I would love to have this as my first contribution in pydotenv let me know if this is possible i may submit a PR. Thanks

@bbc2
Copy link
Collaborator

@bbc2 bbc2 commented Jun 26, 2020

Yes you may submit an PR. Please try to see if the change could break an existing use case. We might have been loading values in that order for a reason. Let me know if you have questions about the implementation or the tests.

@bbc2
Copy link
Collaborator

@bbc2 bbc2 commented Jul 3, 2020

Fixed by #258.

@bbc2 bbc2 closed this Jul 3, 2020
transcranial added a commit to transcranial/python-dotenv that referenced this issue Jul 13, 2020
bbc2 added a commit that referenced this issue Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants