Remove "filthy magic stack backtracking" in favor of BASE_DIR #252
Conversation
|
PR was closed by mistake. Reopened it. |
The read_env documentation currently states: > If not given a path to a dotenv path, does filthy magic stack > backtracking to find manage.py and then find the dotenv. Based on joke2k#88, it appears that this behavior is rather fragile. To improve this, I think the package should document that read_env expects you to provide a path. If one is not provided, it will attempt to use the BASE_DIR constant from the django settings module. If an ImportError is encountered while it attempts to do this, read_env will assume there's no .env file to be found, log an info message to that effect, and continue on. fixes: joke2k#88
The django.conf.settings module could be successfully imported but the BASE_DIR constant could not be defined, which would raise a NameError. This small edit ensures that we're catching both potential failures.
birdcar
force-pushed
the
@nickcannariato-refactor-blank-read-env-import-and-docs
branch
from
279be8a to
05de258
Compare
sergeyklay
left a comment
sergeyklay
left a comment
There was a problem hiding this comment.
There are some linting issues. Could you please take a look
| import logging | ||
| import os | ||
| import re | ||
| import sys |
There was a problem hiding this comment.
'sys' imported but unused
| urlparse, | ||
| urlunparse, | ||
| ) | ||
| from urllib.parse import ParseResult, parse_qs, unquote_plus, urlparse, urlunparse |
There was a problem hiding this comment.
Imported names are in the wrong order
| from urllib.parse import ParseResult, parse_qs, unquote_plus, urlparse, urlunparse | |
| from urllib.parse import parse_qs, ParseResult, unquote_plus, urlparse, urlunparse |
There was a problem hiding this comment.
Also this line too long (82 > 79 characters)
| from urllib.parse import ParseResult, parse_qs, unquote_plus, urlparse, urlunparse | ||
|
|
||
| from .compat import DJANGO_POSTGRES, ImproperlyConfigured, json, REDIS_DRIVER | ||
| from .compat import DJANGO_POSTGRES, REDIS_DRIVER, ImproperlyConfigured, json |
There was a problem hiding this comment.
Imported names are in the wrong order
| from .compat import DJANGO_POSTGRES, REDIS_DRIVER, ImproperlyConfigured, json | |
| from .compat import DJANGO_POSTGRES, ImproperlyConfigured, json, REDIS_DRIVER |
| (True, True) | ||
| >>> root('dev', 'not_existing_dir', required=True) | ||
| Traceback (most recent call last): | ||
| environ.environ.ImproperlyConfigured: Create required path: /home/not_existing_dir |
There was a problem hiding this comment.
E501 line too long (90 > 79 characters)
| >>> public = root.path('public') | ||
| >>> public, public.root, public('styles') | ||
| (<Path:/home/public>, '/home/public', '/home/public/styles') | ||
| >>> assets, scripts = public.path('assets'), public.path('assets', 'scripts') |
There was a problem hiding this comment.
line too long (85 > 79 characters)
| >>> assets.root, scripts.root | ||
| ('/home/public/assets', '/home/public/assets/scripts') | ||
| >>> assets + 'styles', str(assets + 'styles'), ~assets | ||
| (<Path:/home/public/assets/styles>, '/home/public/assets/styles', <Path:/home/public>) |
There was a problem hiding this comment.
line too long (94 > 79 characters)
| try: | ||
| from django.conf import settings | ||
| env_file = os.path.join(settings.BASE_DIR, '.env') | ||
| except (ImportError, NameError): |
There was a problem hiding this comment.
What about AttributeError?
|
@sergeyklay is it possible to change the merge target of this PR to be |
|
@jayvdb Usually we merge changes to the |
|
No worries. At the time I wrote that, there was no |
4 participants
The
read_envdocumentation currently states:Based on #88, it appears that this behavior is rather fragile.
To improve this, I think the package should document that read_env expects you to provide a path. If one is not provided, it will attempt to use the BASE_DIR constant from the django settings module. If an ImportError is encountered while it attempts to do this, read_env will assume there's no .env file to be found, log an info message to that effect, and continue on.
fixes: #88