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

requireFile option for strict checking of env file existence #297

Closed
rptaylor opened this issue Jan 4, 2021 · 6 comments
Closed

requireFile option for strict checking of env file existence #297

rptaylor opened this issue Jan 4, 2021 · 6 comments

Comments

@rptaylor
Copy link

rptaylor commented Jan 4, 2021

In some scenarios (say if you want clear deterministic behaviour for a production app) it is desirable to immediately fail with an obvious error if the specified env file is absent, instead of starting the application in a potentially problematic way due to missing configuration. Clear behaviour, explicit requirements, and obvious failures are important to avoid fragile situations, such as an app working in production by accidentally getting the right config from an unexpected place. Maybe later you clean up other files that you think aren't needed, but then the app breaks because it was not actually getting the config from where you thought it was.

There have been several issues discussing this (both in dotenv and downstream environs):

To provide a solution for these types of problems, I propose a new option, something like "requireFile", for load_dotenv. It can be false by default to preserve current behaviour. @theskumar
If true, it would enable a strict file existence requirement for load_dotenv , so that it fails if no env file is found, no matter how/where (whether it is a named one specified by dotenv_path, or the default ".env" or found by recursing directories ).
And the behaviour of how/where env files are found would remain unchanged. Would that be a reasonable way to address this?

@rptaylor
Copy link
Author

rptaylor commented Jan 4, 2021

#245 this added a warning output if verbose is true, which can help with visibility but doesn't address the issue if you want a strict requirement on config file existence.

@reorx
Copy link

reorx commented Jan 12, 2021

Hey bro, I’m happy to see you have the same opinion with me, but somehow the author does not like this idea, I think you can just fork this repo for your personal use, or just include this single file library as a module of your project/package like I did. I don’t want to be mean here, but this is actually a very simple suggestion with straight forward solution, if he didn’t take my suggestion, he won’t take yours either, I almost feel sorry for you, I just cannot unsee warm-hearted people who want to make reasonable contributions being ignored 😔

@guhcampos
Copy link

This would be useful. For projects heavily relying on environment configuration, such as Kubernetes applications, it may prove useful to ensure the environment has been correctly loaded prior to running tests.

For an app I may have, for example, a few environment files such as integration.env, regression.env, staging.env and I want to make sure they are properly loaded during my pytest configuration hooks. Also, I want to make sure that any of my own development environment variables will be overriden by the loaded values, just so I don't mistakenly run tests using my own environment.

I had initially assumed load_dotenv would return False if it never found the provided environment file, but it turns out that:

def pytest_configure(*_):
    x = load_dotenv(Path("gibberish") / "tests.env", override=True)

gives me x=True.

It would be perfectly sufficient if it returned False, which seems like a less intrusive option if the new parameter does not suit everyone's needs.

@theskumar
Copy link
Owner

We have a similar request here:

I like the approach of using exceptions after passing an additional parameter, raise_exception=True. As this library is still not v1, I think we would have a chance to make a default behavior as well. For now, I think should be good with an additional parameter.


It would be perfectly sufficient if it returned False, which seems like a less intrusive option if the new parameter does not suit everyone's needs.

@guhcampos This is now added to the latest code. (Not released yet).

@rptaylor
Copy link
Author

Looks like something related was implemented in #388

Instead of the raise_exception=True parameter it looks like users will have to check the boolean return code.

@rptaylor
Copy link
Author

Also there isn't a warning if Verbose is on and the file doesn't exist:

import dotenv
a=dotenv.load_dotenv('no.env', verbose=True)
print(a)
False

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

4 participants