make DOTENV setting more robust

[P1roks]

While adding django-configuratons to a big project that I'm working on I've noticed a few issues with DOTENV implementation and this PR focuses on fixing them, while still allowing for the old way of doing this. They are:

• always failing if DOTENV file is not found (this was faulty due to CI not having this .env)
• using setdefault to load variables (this was faulty because in a big project a lot of environmental variables come from a lot of sources and it's sometimes desirable to override them via DOTENV)
• DOTENV not reloading on hotreload (this is annoying when you have to restart a docker container to see any changes that you've made in DOTENV)

[uhurusurfa]

Than ks for. the contribution and sorry it has taken so long for this to be reviewed.
Can you add a small unit test to the tests/test_env.py for using the DOTENV as a dictionary?

[P1roks]

Than ks for. the contribution and sorry it has taken so long for this to be reviewed. Can you add a small unit test to the tests/test_env.py for using the DOTENV as a dictionary?

Oh wow. I completely forgot about this PR, but I've written a quick test just now and added it to tests/test_env.py as you asked me to

[uhurusurfa]

Thanks for jumping on that so quickly.
Think just the one noted issue should solve the Actions failures

[uhurusurfa]

This should be from tests.settings import tests.settings.dot_env_dict

[P1roks]

Yeah that was a silly oversight. I would normally run test locally but I'm currently on NixOS and don't have time to read how to use tox with it

[codecov]

Codecov Report

Attention: Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 90.62%. Comparing base (cad6dcb) to head (bc11925).
Report is 22 commits behind head on master.

Files with missing lines	Patch %	Lines
configurations/base.py	85.71%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #364      +/-   ##
==========================================
+ Coverage   89.98%   90.62%   +0.64%     
==========================================
  Files          25       27       +2     
  Lines        1198     1227      +29     
  Branches      216       89     -127     
==========================================
+ Hits         1078     1112      +34     
+ Misses         89       87       -2     
+ Partials       31       28       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[uhurusurfa]

These flake8 issues need sorting:

  ./configurations/base.py:114:29: W291 trailing whitespace
  ./configurations/base.py:121:93: W291 trailing whitespace
  ./tests/settings/dot_env_dict.py:9:6: W291 trailing whitespace

[uhurusurfa]

Lookxs like we need to add a test that passes an invalid/non-existent .env file in so that the OSError exception is triggered to achieve the desired code coverage set on this project.

[P1roks]

I've added the opposite test since DOTENV being required is the default behavior. It tries to use nonexistent DOTENV path and since required is set to False it shouldn't raise an exception

[uhurusurfa]

@pauloxnet - are you happy for this to be merged now?

[uhurusurfa]

@cclauss - do you have any comments/issues with this PR being merged?

[uhurusurfa]

Looks good to me

[pauloxnet]

It seems a change of behaviors to me.
I don't have right now the time to do a deep down review myself, nut I would ask for more consensus in the community here before merge it

[uhurusurfa]

It seems a change of behaviors to me. I don't have right now the time to do a deep down review myself, nut I would ask for more consensus in the community here before merge it

Can you provide details on how you think it has changed please?
From my perspective, it is simply an enhancement allowing the existing behaviour to continue burt with the option of an enhanced way of managing the environment variable insertion.

Furthermore, there is no change to any unit tests so I am struggling to see how this has affected existing functionality.
@cclauss - do you have any opinions on this?