Skip to content

dotenv_load with bad file path doesn't error #164

Description

@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

Activity

  1. bbc2 commented on Jan 28, 2019

    @bbc2
    Collaborator

    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.

  2. bandtank commented on Jan 28, 2019

    @bandtank
    Author

    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.

  3. kvfi commented on Mar 17, 2019

    @kvfi

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

  4. theskumar commented on May 22, 2019

    @theskumar
    Owner

    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
    
  5. drkarthi commented on Jan 29, 2020

    @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?

  6. bandtank commented on Jan 29, 2020

    @bandtank
    Author

    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.

  7. drkarthi commented on Jan 29, 2020

    @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.

  8. bbc2 commented on Jan 29, 2020

    @bbc2
    Collaborator

    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.

  9. bbc2 commented on Jan 29, 2020

    @bbc2
    Collaborator

    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.

  10. srgykuz commented on Mar 17, 2020

    @srgykuz

    +1 for that issue.

  11. mbUSC commented on Jul 7, 2023

    @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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions