dotenv_load with bad file path doesn't error

[bandtank]

Dotenv accepts a bad file name as dotenv_path and returns True even though the file could not have been loaded because it doesn't exist. I am now checking for existence of the file ahead of time, but the library really should check this on its own. Libraries that interact with files should throw exceptions when the files are inaccessible.

$ python
Python 3.7.2 (default, Jan 24 2019, 09:21:54) 
[GCC 8.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import dotenv
>>> e = dotenv.find_dotenv()
'/home/<user>/repositories/<repo>/.env'
>>> dotenv.load_dotenv(dotenv_path = e)
True
>>> dotenv.load_dotenv(dotenv_path = "/tmp/badFileName")
True
>>> quit()
$ ll /tmp/badFileName
ls: cannot access '/tmp/badFileName': No such file or directory

[bbc2]

Thank you for reporting this issue. I observe the same behavior as you. I had a look at the code and it's pretty obvious that True is the only value that load_dotenv can return. This indeed looks like a bug.

What load_dotenv should return is not documented as far as I know, but there are two tests that suggest it should return whether a .env file was found as a boolean.

Your suggestion - raising an exception if the file wasn't found - is a good idea in general. However, it's normal for a .env file to be absent (e.g. in staging or in production), so I would say the boolean approach makes more sense here.

[bandtank]

A boolean return value to indicate success or failure would be helpful. It looks like that's hardcoded to True in this function. I don't understand how the class works, so I'm not sure how that function ever interacts with the file name/path.

[kvfi]

It actually does throw an exception if you set verbose to True.

[theskumar]

verbose=True displays warning if file was not found.

The return value of load_dotenv is undocumented as I was planning to do something useful with it, but could not settle down to one.

I think it make sense to return proper return based on if file was found or not, but if the idea is to just stop execution if no .env is found I think it would make much more sense to just pass it a fail_silently=False and have it raise an exception.

It would be great to return a getter object/function as return value of load_dotenv(). That object then can be used to get parsed values from env.

e.g.

env = load_dotenv(fail_silently=False)
DEBUG = env("DEBUG", cast=bool, default=False)

It may also contain some of the meta info like file_path.

>> print(env.file_path)
/etc/config/.env

[drkarthi]

I can make a PR for this! From the above discussion, my understanding is we want to have an option to fail silently or with an exception. If it fails silently, we want to reflect the status and other metadata in the return value. Do we want to do this irrespective of the value of verbose or just when verbose=True?

[bandtank]

My preference would be stop execution if the env file is not found, but to also be able to silently fail if desired. In general, silent failures aren't a great idea, but I think it should be an option considering the use cases for dotenv. Regardless, knowing what happened is the key whether or not verbose is set to true, which shouldn't be a requirement to know if something failed in my opinion.

[drkarthi]

I agree, my opinion is also that it should be independent of the verbose value. For whether we want an exception or a silent failure to be the default, I would like to hear more opinions. I do not use it for web apps and do not have an opinion on what the default should be for that use-case.

[bbc2]

Thanks to #211, errors are not silent anymore. Does that work for you?

However, there's still no way to stop execution when syntax errors are found in the .env file. I think a strict option would make sense. Such an option would be false by default at the beginning, so that it wouldn't break existing applications.

Returning whether a .env was loaded as a boolean probably also makes sense. As mentioned earlier, I don't think we want to raise anything, not even a warning, when a .env file isn't found, since it's normal for the file to be missing in production.

[bbc2]

Never mind my initial question about syntax warnings. It's different from the "file not found" question. I mixed up those two types of errors.

[srgykuz]

+1 for that issue.

[mbUSC]

Sad :(

Having used many other Python libraries, I personally find this behavior to be unexpected and confusing.
It would make much more sense to throw an exception if the .env file does not exist