#32664 closed Cleanup/optimization (fixed)
Cannot run makemigrations management command without a SECRET_KEY
Reported by: | François Freitag | Owned by: | François Freitag |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Florian Apolloner, François Freitag | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I believe #29324 intended to fix this issue.
Steps to reproduce:
$ cd $(mktemp -d) $ python -m venv venv $ source venv/bin/activate $ pip install 'Django>=3.2' $ python -m django startproject foo $ sed -ri '/SECRET_KEY/d' foo/foo/settings.py # Remove SECRET_KEY from settings $ PYTHONPATH=foo DJANGO_SETTINGS_MODULE="foo.settings" python -m django makemigrations --check
The output is attached.
Attachments (1)
Change History (14)
by , 4 years ago
comment:1 by , 4 years ago
Cc: | added |
---|---|
Resolution: | → needsinfo |
Status: | assigned → closed |
comment:2 by , 4 years ago
Resolution: | needsinfo |
---|---|
Status: | closed → new |
I am using the makemigrations
command with the --check
toggle to verify no new migrations are needed. I don’t think it needs a SECRET_KEY
?
comment:3 by , 4 years ago
Has patch: | set |
---|
Here’s a patch that solves the issue for me: https://github.com/django/django/pull/14282
That may help describe and reproduce the issue.
comment:4 by , 4 years ago
I'm not sure about fixing this. It's a reasonable assumption that SECRET_KEY
is necessary for all built-in commands. We cannot not guarantee that makemigrations
or any other built-in command will work in the future without a SECRET_KEY
.
comment:5 by , 4 years ago
We cannot not guarantee that makemigrations or any other built-in command will work in the future without a SECRET_KEY.
That’s true. I’m not asking for all management commands to work without a SECRET_KEY
, but for the commands to fail only when the SECRET_KEY
is accessed.
My goal is to avoid defining a SECRET_KEY
in environments that do not need it. That’s the same goal as #29324 and corresponding release note mention https://docs.djangoproject.com/en/3.2/releases/3.2/#security:
running management commands that do not rely on the SECRET_KEY without needing to provide a value.
My project (the same as Jon in #29324) works around the limitation by generating a random string for SECRET_KEY
when none is available. It is annoying to maintain logic to create values for unused settings.
A regression in this area isn’t a big deal. IMO, maintaining it on a best-effort basis is sufficient, and can help simplifying the running on management commands for other projects.
comment:6 by , 4 years ago
Cc: | added |
---|
comment:7 by , 4 years ago
I think the proper fix would be to move PasswordResetTokenGenerator.secret
to a property so it is only accessed when actually needed. This would also help tests that change the SECRET_KEY
and then wonder why default_token_generator
does not pick it up. Lazy*
should be used as a last resort.
comment:8 by , 4 years ago
Thanks for the suggestion, it is a good improvement!
I updated the patch https://github.com/django/django/pull/14282 to reflect it.
Please let me know if that’s what you had in mind?
comment:9 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
comment:10 by , 4 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
follow-up: 13 comment:12 by , 3 years ago
Considering Django 3.2 release notes mention:
The
SECRET_KEY
setting is now checked for a valid value upon first access, rather than when settings are first loaded. This enables running management commands that do not rely on theSECRET_KEY
without needing to provide a value. As a consequence of this, callingconfigure()
without providing a validSECRET_KEY
, and then going on to accesssettings.SECRET_KEY
will now raise anImproperlyConfigured
exception.
Has it been considered to port this fix to the 3.2 branch? As it is today, PasswordResetTokenGenerator
initialization breaks the execution of management commands when trying to follow the recommendation provided by the release notes.
comment:13 by , 3 years ago
Replying to Michael Manganiello:
Has it been considered to port this fix to the 3.2 branch? As it is today,
PasswordResetTokenGenerator
initialization breaks the execution of management commands when trying to follow the recommendation provided by the release notes.
This release note is rather about custom management commands. It's arguable (as you see from the above discussion) that build-in commands that rely on django.contrib.auth
module should not require the SECRET_KEY
. However, we decided to do the best we can to make SECRET_KEY
validation lazy also for built-in management commands. This ticket is a cleanup not a bug in 948a874425e7d999950a8fa3b6598d9e34a4b861 that's why it doesn't qualify for a backport.
#29324 fix this issue for management commands that do not rely on the
SECRET_KEY
, as far as I'm awarecheck
is not one of them. Have you tried with a custom management command?