Allow existing env vars to be updated

[codeinthehole]

We're using envparse in a Django project that uses consul-template to provide the env file. When config is updated in Consul, the env file is updated and we touch the WSGI file to (attempt to) pick up the new configuration.

However this doesn't work as env.read_envfile uses os.environ.setdefault so updates are never applied (as the env var already exists).

What's the reasoning for using setdefault instead of just setting directly. If that can't be changed, would you be open to passing an additional flag to read_envfile to allow updates? Am happy to write pull request.

[rconradharris]

I don't think I wrote that bit of code, but I believe the idea here was that manually set environment variables should take precedence over variables specified in a file. For example, if I have a test program foo that reads from a file that specifies MYKEY=myval, and then i run foo as:

$ MYKEY=differentval foo

The principle of least surprise would dictate that differentval overrides the default specified in the file.

It seems like in your case, you'd want a refresh=True kwarg.

A couple of issues here:

1) Environment variables aren't really designed to be changed at run-time. I think the 12-factor app-ish suggestion would be that a new process with a new environment should be forked off and used. But, in your case, that might not be possible or make sense.

2) The API design of read_envfile is a little wonky in theat **overrides captures all of the kwargs, so just adding a refresh=True environment variable will break backwards compatibility. Not a huge issue though, since that probably should be fixed any way: I'm not really sure why overrides need to be specified this way (again, I don't think I wrote this bit of code).

I'm open to suggestions on what you think is best here. Thanks!

[codeinthehole]

Good point about how the existing implementation ensures manually set env vars take precedence.

You're right too about the 12-factor approach. The trouble is we sometimes want to quickly tweak a setting and see the results without waiting for a new set of servers to deploy (which takes a few minutes).

2) The API design of read_envfile is a little wonky in that **overrides captures all of the kwargs, so just adding a refresh=True environment variable will break backwards compatibility. Not a huge issue though, since that probably should be fixed any way: I'm not really sure why overrides need to be specified this way (again, I don't think I wrote this bit of code).

Changing:

def read_envfile(path=None, **overrides):

to

def read_envfile(path=None, refresh=False, **overrides):

only breaks compatibility for people already using an refresh override right? That doesn't seem too onerous (although easy for me to say - no-one will complain to me if this breaks someone's application).

The only other option would be a new function/method, perhaps refresh_from_file, that does the same as read_envfile but ignores existing env vars.

Any preference?

[rconradharris]

Thinking a bit more about this.

Seems like the name refresh=True isn't really that descriptive of the general case; for example makes less sense if you're just running this once.

_overwrite=False seems a little better since it describes what's happening: we're overwriting any existing values with what's in the file; and we're not colliding if someone happens to use OVERRIDE=1 in their app.

In a 1.0 release, I think I'd want to remove that unnecessary **overrides argument.

[codeinthehole]

Ok, I'll submit a PR with that change.

[crucialfelix]

This has a second implication:

environ.Env.read_env(root('envs/common.env'))
environ.Env.read_env(root('envs/local_settings.env'))

I expected that local_settings would overwrite values set in common. But if a value is set in common then it's already set and read_env('local_settings.env') has no effect.

So to get the behavior I expect (merging two env files) I have to load them in reverse order :0