Fixed #29324 -- Made Settings raise ImproperlyConfigured on SECRET_KEY access; not in init. #9876
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
I see your point. I've updated the change to now do both:
Thanks! |
timgraham
reviewed
Apr 16, 2018
There was a problem hiding this comment.
This message changed slightly:
diff --git a/tests/settings_tests/tests.py b/tests/settings_tests/tests.py
index 5a61895..09c062c 100644
--- a/tests/settings_tests/tests.py
+++ b/tests/settings_tests/tests.py
@@ -234,7 +234,8 @@ class SettingsTests(SimpleTestCase):
settings.TEST = 'test'
self.assertEqual('test', settings.TEST)
del settings.TEST
- with self.assertRaises(AttributeError):
+ msg = "'Settings' object has no attribute 'TEST'"
+ with self.assertRaisesMessage(AttributeError, msg):
getattr(settings, 'TEST')
def test_settings_delete_wrapped(self):I don't know that it matters either way, but please test it.
django/conf/__init__.py
Outdated
| @@ -115,15 +115,15 @@ def __init__(self, settings_module): | |||
| if setting.isupper(): | |||
| setting_value = getattr(mod, setting) | |||
|
|
|||
| if setting == 'SECRET_KEY' and not setting_value: | |||
| raise ImproperlyConfigured("The SECRET_KEY setting must not be empty.") | |||
docs/ref/settings.txt
Outdated
| @@ -2120,6 +2122,12 @@ affect them. | |||
| startproject <startproject>` creates a unique ``SECRET_KEY`` for | |||
| convenience. | |||
|
|
|||
| .. versionchanged:: 2.1 | |||
|
|
|||
| ``SECRET_KEY`` is no longer defined by default. Django will now raise an | |||
There was a problem hiding this comment.
I think this is sufficient, "In older versions, SECRET_KEY defaults to an empty string." (use 4 space indent)
docs/ref/settings.txt
Outdated
| @@ -2087,7 +2087,9 @@ Uses of the key shouldn't assume that it's text or bytes. Every use should go | |||
| through :func:`~django.utils.encoding.force_text` or | |||
| :func:`~django.utils.encoding.force_bytes` to convert it to the desired type. | |||
|
|
|||
| Django will refuse to start if :setting:`SECRET_KEY` is not set. | |||
| Django will refuse to start if :setting:`SECRET_KEY` is set to an empty value. | |||
| An :class:`~django.core.exceptions.ImproperlyConfigured` exception will be | |||
There was a problem hiding this comment.
is raised if SECRET_KEY is accessed but not set.
tests/admin_scripts/tests.py
Outdated
| out, | ||
| "+ INSTALLED_APPS = ['django.contrib.auth', 'django.contrib.contenttypes', 'admin_scripts']" | ||
| ) | ||
| self.assertNotInOutput(out, "- SECRET_KEY = ''") |
There was a problem hiding this comment.
I think this assertion isn't needed.
|
Thanks for the review Tim. I've made the suggested changes. |
…Y is accessed and not set.
beniwohli
added a commit
to beniwohli/apm-agent-python
that referenced
this pull request
Apr 26, 2018
django/django#9876 removed the default SECRET_KEY, leading to an error when we try to access it in the tests
beniwohli
added a commit
to elastic/apm-agent-python
that referenced
this pull request
Apr 26, 2018
django/django#9876 removed the default SECRET_KEY, leading to an error when we try to access it in the tests closes #204
beniwohli
added a commit
to elastic/apm-agent-python
that referenced
this pull request
Apr 26, 2018
django/django#9876 removed the default SECRET_KEY, leading to an error when we try to access it in the tests closes #204
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://code.djangoproject.com/ticket/29324