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

load_dotenv() returns True even if .env file is not found #321

Closed
scollovati opened this issue May 9, 2021 · 6 comments · Fixed by #388
Closed

load_dotenv() returns True even if .env file is not found #321

scollovati opened this issue May 9, 2021 · 6 comments · Fixed by #388

Comments

@scollovati
Copy link

scollovati commented May 9, 2021

This behaviour is not ideal since if the configuration file is not found in the filesystem it may be better to return False.
In this way the user can write something like:

if load_dotenv():
    #do stuff
else
    print("No file .env found")

Steps to reproduce

In [1]: import dotenv

In [2]: dotenv.load_dotenv()
Out[2]: True

In [3]: dotenv.find_dotenv()
Out[3]: ''
@theskumar
Copy link
Owner

I understand the return response for load_dotenv is not ideal, but at the same time as load_dotenv just not accepts a file on the filesystem but a StringIO as well, so the return value indicating the existing of a file doesn't make any sense.

There are other ways to get around this for you use case.

  1. Enable verbose=True in load_dotenv to get a error log if file is not found.
  2. Use the find_dotenv function to check if the file exist or not or simply check the file yourself, before loading the values from .env.

It feel like exposing an instance of Dotenv class might make sense here or None if nothing is processed by load_dotenv. cc: @bbc2 thoughts?

@bbc2
Copy link
Collaborator

bbc2 commented Jul 3, 2021

so the return value indicating the existing of a file doesn't make any sense.

I think it would be OK to return True if a StringIO was provided. False would still be useful for users using files.

So I'd suggest implementing that, or changing strategies and adding a raise_if_file_not_found argument to load_dotenv and dotenv_values. One or the other, but not both.

It feel like exposing an instance of Dotenv class might make sense here or None if nothing is processed by load_dotenv.

Although it could lead to interesting features, I'm worried that exposing the Dotenv class would increase our API surface considerably. I kind of like the fact that we can change that class easily at the moment, which wouldn't be the case if we exposed it to users.

About returning None, what would be the advantage compared to returning False?

@iamsannyrai
Copy link

@theskumar why we can access values from .env files by just os.getenv("VARIABLE_NAME") without even calling load_env() ? Any reason?

@ethsanders
Copy link

@theskumar why we can access values from .env files by just os.getenv("VARIABLE_NAME") without even calling load_env() ? Any reason?

load_env sets the environment variables of the python instance to the ones found in the file. If you don't call I, then it uses the local variables of the system.

larsks added a commit to larsks/python-dotenv that referenced this issue Mar 15, 2022
This modifies Dotenv.set_as_environment_variables to return False
if we have not discovered any environment variables via either
`dotenv_path` or `stream`.

This return value gets passed through to `load_dotenv`, so this can be
used to determine if `dotenv.load_dotenv` was able to set anything.

Closes theskumar#321
larsks added a commit to larsks/python-dotenv that referenced this issue Mar 15, 2022
This modifies Dotenv.set_as_environment_variables to return False
if we have not discovered any environment variables via either
`dotenv_path` or `stream`.

This return value gets passed through to `load_dotenv`, so this can be
used to determine if `dotenv.load_dotenv` was able to set anything.

Closes theskumar#321
larsks added a commit to larsks/python-dotenv that referenced this issue Mar 15, 2022
This modifies Dotenv.set_as_environment_variables to return False
if we have not discovered any environment variables via either
`dotenv_path` or `stream`.

The return value gets passed through to `load_dotenv`, so this can be
used to determine if `dotenv.load_dotenv` was able to set anything.

Closes theskumar#321
@larsks
Copy link
Contributor

larsks commented Mar 15, 2022

I've submitted a PR that changes the behavior of dotenv.Dotenv.set_as_environment_variables such that load_dotenv will return False if it is unable to load any environment variables from a file or stream. I think that addresses the concerns raised in this issue.

larsks added a commit to larsks/python-dotenv that referenced this issue Mar 15, 2022
This modifies Dotenv.set_as_environment_variables to return False
if we have not discovered any environment variables via either
`dotenv_path` or `stream`.

The return value gets passed through to `load_dotenv`, so this can be
used to determine if `dotenv.load_dotenv` was able to set anything.

Closes theskumar#321
larsks added a commit to larsks/python-dotenv that referenced this issue Mar 15, 2022
This modifies Dotenv.set_as_environment_variables to return False
if we have not discovered any environment variables via either
`dotenv_path` or `stream`.

The return value gets passed through to `load_dotenv`, so this can be
used to determine if `dotenv.load_dotenv` was able to set anything.

Closes theskumar#321
@rptaylor
Copy link

I think this is the same or similar to #297 ?

theskumar added a commit that referenced this issue Jun 5, 2022
…388)

* Fix docstring for load_dotenv

The docstring for load_dotenv was missing a word, rendering it confusing.
This commits modifies it for clarity.

* Return False when we do not discover any environment variables

This modifies Dotenv.set_as_environment_variables to return False
if we have not discovered any environment variables via either
`dotenv_path` or `stream`.

The return value gets passed through to `load_dotenv`, so this can be
used to determine if `dotenv.load_dotenv` was able to set anything.

Closes #321

Co-authored-by: Saurabh Kumar <theskumar@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants