Allow existing env vars to be updated about envparse

[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