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

Make .env file optional in read_env method #243

Closed
afilardo opened this issue Nov 19, 2019 · 4 comments · Fixed by #251
Closed

Make .env file optional in read_env method #243

afilardo opened this issue Nov 19, 2019 · 4 comments · Fixed by #251
Assignees
Labels
bug Something isn't working

Comments

@afilardo
Copy link

Hi,

I think that this proposal is interesting because I can imagine that more devs have been in the same situation multiple times:

  • I want to use .env file when working locally.
  • But it is a bad practice to push .env file to GitHub.
  • I have a SECRET_KEY that it is required. If it is no present, Django app will raise ImproperlyConfigured, which is fine.
  • I don't want to set this env vars in my computer each time (that is why we use .env file).
  • But I know that it is mandatory in PROD env, which is fine too.

Having an optional .env file will allow us to have the read_env method in our base_settings file, which will work in PROD env too (where we don't have .env file).

The read_env method should read the file (if present) and add their vars to the environment scope.
If the file is not present and I forgot to set any var, Django app will raise the ImproperlyConfigured exception.
I think there is no risk in making this file optional.
At this moment, Django app does not run if the .env file is not present.

This is just my opinion. I would like to know how you handle this situation, and what do you think about my proposal.

Regards

@afilardo afilardo changed the title Make _.env_ file optional in _read_env_ method Make .env file optional in read_env method Nov 19, 2019
@herrboyer
Copy link

As a workaround you could

PROJECT_ROOT = environ.Path(__file__) - 3
try:
    PROJECT_ROOT.file(".env")
    environ.Env.read_env(PROJECT_ROOT(".env"))
except FileNotFoundError:
    pass

@afilardo
Copy link
Author

afilardo commented Dec 3, 2019

Yeah, it is what I'm using, but I find it unnecessary

@eedwards-sk
Copy link

I agree with OP, since django-environ should not get in the way of 12factor configuration. Requiring an .env file breaks that contract.

birdcar added a commit to birdcar/django-environ that referenced this issue Mar 19, 2020
Currently, if a `.env` file isn't found by the read_env() classmethod,
it raises a UserWarning. Based on Python's logging guide,
warnings.warn() is only appropriate to be raised in library code if the
issue is avoidable and the client application should be modified to
eliminate the warning.

Conversely, it recommends logging.warning() if there is nothing the
client application can do about the situation, but the event should
still be noted (see ref1).

In this pariticular case, the function is working with `.env` files that
should only ever exist in development environments, which means that
the function will erroneously warn the consumer in production and
staging environments.

With this in mind, I believe the most sensible thing to do would be to
convert read_env from using the warnings module to using the logging
module. Additionally, since the lack of a `.env` file is going to occur
regularly during normal operation of the program, I also believe that
this should be downgraded to an INFO level message from a WARNING
level message.

joke2k#243 requests that the .env file be made
optional, and as far as I can the .env file is indeed optional, it's
just that a UserWarning is being raised instead of a logged message.
With that in mind, I'm referencing it as being closed by this change.

ref1: https://docs.python.org/3/howto/logging.html#when-to-use-logging

fixes: joke2k#243
birdcar added a commit to birdcar/django-environ that referenced this issue Mar 19, 2020
Currently, if a `.env` file isn't found by the read_env() classmethod,
it raises a UserWarning. Based on Python's logging guide,
warnings.warn() is only appropriate to be raised in library code if the
issue is avoidable and the client application should be modified to
eliminate the warning.

Conversely, it recommends logging.warning() if there is nothing the
client application can do about the situation, but the event should
still be noted (see ref1).

In this particular case, the function is working with `.env` files that
should only ever exist in development environments, which means that
the function will erroneously warn the consumer in production and
staging environments.

With this in mind, I believe the most sensible thing to do would be to
convert read_env from using the warnings module to using the logging
module. Additionally, since the lack of a `.env` file is going to occur
regularly during normal operation of the program, I also believe that
this should be downgraded to an INFO level message from a WARNING
level message.

joke2k#243 requests that the .env file be made
optional, and as far as I can the .env file is indeed optional, it's
just that a UserWarning is being raised instead of a logged message.
With that in mind, I'm referencing it as being closed by this change.

ref1: https://docs.python.org/3/howto/logging.html#when-to-use-logging

fixes: joke2k#243
birdcar added a commit to birdcar/django-environ that referenced this issue Mar 19, 2020
Currently, if a `.env` file isn't found by the read_env() classmethod,
it raises a UserWarning. Based on Python's logging guide,
warnings.warn() is only appropriate to be raised in library code if the
issue is avoidable and the client application should be modified to
eliminate the warning.

Conversely, it recommends logging.warning() if there is nothing the
client application can do about the situation, but the event should
still be noted (see ref1).

In this particular case, the function is working with `.env` files that
should only ever exist in development environments, which means that
the function will erroneously warn the consumer in production and
staging environments.

With this in mind, I believe the most sensible thing to do would be to
convert read_env from using the warnings module to using the logging
module. Additionally, since the lack of a `.env` file is going to occur
regularly during normal operation of the program, I also believe that
this should be downgraded to an INFO level message from a WARNING
level message.

joke2k#243 requests that the .env file be made
optional, and as far as I can the .env file is indeed optional, it's
just that a UserWarning is being raised instead of a logged message.
With that in mind, I'm referencing it as being closed by this change.

ref1: https://docs.python.org/3/howto/logging.html#when-to-use-logging

fixes: joke2k#243
birdcar added a commit to birdcar/django-environ that referenced this issue Sep 5, 2021
Currently, if a `.env` file isn't found by the read_env() classmethod,
it raises a UserWarning. Based on Python's logging guide,
warnings.warn() is only appropriate to be raised in library code if the
issue is avoidable and the client application should be modified to
eliminate the warning.

Conversely, it recommends logging.warning() if there is nothing the
client application can do about the situation, but the event should
still be noted (see ref1).

In this particular case, the function is working with `.env` files that
should only ever exist in development environments, which means that
the function will erroneously warn the consumer in production and
staging environments.

With this in mind, I believe the most sensible thing to do would be to
convert read_env from using the warnings module to using the logging
module. Additionally, since the lack of a `.env` file is going to occur
regularly during normal operation of the program, I also believe that
this should be downgraded to an INFO level message from a WARNING
level message.

joke2k#243 requests that the .env file be made
optional, and as far as I can the .env file is indeed optional, it's
just that a UserWarning is being raised instead of a logged message.
With that in mind, I'm referencing it as being closed by this change.

ref1: https://docs.python.org/3/howto/logging.html#when-to-use-logging

fixes: joke2k#243
@sergeyklay sergeyklay added the bug Something isn't working label Sep 6, 2021
@sergeyklay sergeyklay self-assigned this Sep 6, 2021
@sergeyklay
Copy link
Collaborator

This is resolved in develop branch. Thank you for the report, and for helping us make django-environ better. And I am sorry about the delay.

gcf-merge-on-green bot pushed a commit to GoogleCloudPlatform/python-docs-samples that referenced this issue Sep 14, 2021
[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [django-environ](https://django-environ.readthedocs.org) ([source](https://togithub.com/joke2k/django-environ), [changelog](https://django-environ.readthedocs.org/en/latest/changelog.html)) | `==0.6.0` -> `==0.7.0` | [![age](https://badges.renovateapi.com/packages/pypi/django-environ/0.7.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/django-environ/0.7.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/django-environ/0.7.0/compatibility-slim/0.6.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/django-environ/0.7.0/confidence-slim/0.6.0)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>joke2k/django-environ</summary>

### [`v0.7.0`](https://togithub.com/joke2k/django-environ/blob/master/CHANGELOG.rst#v070---11-September-2021)

[Compare Source](https://togithub.com/joke2k/django-environ/compare/v0.6.0...v0.7.0)

Added
\+++++

-   Added support for negative float strings
    `#&#8203;160 <https://github.com/joke2k/django-environ/issues/160>`\_.
-   Added Elasticsearch5 to search scheme
    `#&#8203;297 <https://github.com/joke2k/django-environ/pull/297>`\_.
-   Added Elasticsearch7 to search scheme
    `#&#8203;314 <https://github.com/joke2k/django-environ/issues/314>`\_.
-   Added the ability to use `bytes` or `str` as a default value for `Env.bytes()`.

Fixed
\+++++

-   Fixed links in the documentation.
-   Use default option in `Env.bytes()`
    `#&#8203;206 <https://github.com/joke2k/django-environ/pull/206>`\_.
-   Safely evaluate a string containing an invalid Python literal
    `#&#8203;200 <https://github.com/joke2k/django-environ/issues/200>`\_.

Changed
\+++++++

-   Added 'Funding' and 'Say Thanks!' project urls on pypi.
-   Stop raising `UserWarning` if `.env` file isn't found. Log a message with
    `INFO` log level instead `#&#8203;243 <https://github.com/joke2k/django-environ/issues/243>`\_.

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Never, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/GoogleCloudPlatform/python-docs-samples).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants