#25304 closed New feature (fixed)
Allow management commands to check if database migrations are applied
Reported by: | Maxime Lorant | Owned by: | Mounir |
---|---|---|---|
Component: | Core (Management commands) | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
When creating a new project, you can sometimes forget to run manage.py migrate
before creating the initial superuser (especially if you don't execute runserver
before, which display a warning about migrations not applied). The resulting error make sense, it can't access to auth_user, since it does not exist yet:
$ django-admin.py startproject sample $ cd sample/ && python manage.py createsuperuser Traceback (most recent call last): File "manage.py", line 10, in <module> execute_from_command_line(sys.argv) [...] File "/vagrant/django/django/contrib/auth/management/commands/createsuperuser.py", line 85, in handle default_username = get_default_username() File "/vagrant/django/django/contrib/auth/management/__init__.py", line 189, in get_default_username auth_app.User._default_manager.get(username=default_username) [...] File "/vagrant/django/django/db/backends/sqlite3/base.py", line 323, in execute return Database.Cursor.execute(self, query, params) django.db.utils.OperationalError: no such table: auth_user
... but with a little try/except, it could be nicer and give a more meaningful information:
$ python manage.py createsuperuser CommandError: You must execute `manage.py migrate` once before creating a super user
I have a patch ready (as I said, it just a try/except, see attachment) but, if accepted, it would need unit tests to be complete.
Attachments (1)
Change History (16)
by , 9 years ago
Attachment: | try-except-createsuperuser.diff added |
---|
comment:1 by , 9 years ago
Patch needs improvement: | set |
---|
comment:2 by , 9 years ago
I don't see much advantage to this. In my opinion, a developer should be able to understand what the lack of a database table means. There are other management commands that access the database -- do they all need to be adjusted too?
comment:3 by , 9 years ago
I agree with you, a developer should understand that a missing table on a Django default command means the database has not been created yet. However, I sometimes answer to question on StackOverflow or other programming forums even more... easy *cough*... (like "Why my page is not working and throw name 'url' is not defined
in urls.py").
After some thoughts, the idea to raise the exception again with a hint seems more friendly for both usages (new Django users and skilled one), but it is not urgent at all. Also, it needs to establish a list of commands to improve...
comment:4 by , 9 years ago
Component: | contrib.auth → Core (Management commands) |
---|---|
Easy pickings: | unset |
Has patch: | unset |
Patch needs improvement: | unset |
Summary: | Trying to create a super user before first migrate raise a database error → Allow management commands to check if database migrations are applied |
Triage Stage: | Unreviewed → Accepted |
Type: | Cleanup/optimization → New feature |
I don't think the exception catching solution would work perfectly. For example, there's no guarantee that the database exception is due to missing tables and not some other bad query, etc. A more viable solution might be to allow management commands to check if all database migrations are applied and to output a warning if not, similar to what runserver
does. Accepting the ticket for investigation of that idea. #24484 is similar.
comment:6 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 9 years ago
Description: | modified (diff) |
---|---|
Has patch: | set |
comment:8 by , 9 years ago
Description: | modified (diff) |
---|
comment:9 by , 9 years ago
Needs documentation: | set |
---|---|
Patch needs improvement: | set |
Tests aren't running on the pull request and docs are needed.
comment:11 by , 9 years ago
Needs documentation: | unset |
---|---|
Patch needs improvement: | unset |
comment:12 by , 9 years ago
Needs tests: | set |
---|
Could use an additional test as noted on the pull request.
comment:13 by , 9 years ago
Needs tests: | unset |
---|
Hi,
Thanks for filing this report and patch. I think the patch is not an improvement, though:
OperationalError
in different circumstances (e.g. the database file cannot be written to because of permissions or non-existing directory or device full or whatever)The error message, in general, could be made friendlier, but the source error must not be hidden. I think we could accept a similar modification where the
except:
clause catchesDatabaseError
, prints a hint along the lines of "Have you run initial migrations for this project?", and re-raises the exception.For a test, I think you should try to temporarily swap out the user model; that should create the invalid code-vs-database state to trigger the exception.