Migrations and System Checks

[Mark Lavin]

Last weekend I set out to fix https://code.djangoproject.com/ticket/21856 

which was a 1.7 release blocker and holding up additional 1.7 testing for a project

I'm currently working on. Upon diving in I saw that the checks for un-applied migrations

were tied to the runserver command. I thought to myself that this

seemed more appropriate for the new system checks framework. I assumed that it

hadn't been done this way originally because Andrew's work on migrations was running

parallel to the system checks. I figured while I was fixing the issue at hand it

would make sense to migrate (if you'll excuse the pun) this check to the new framework. 

After all the stated goal of these checks was to detecting common problems 

(such as un-applied migrations) and give hints on how to fix them (such as "you should run migrate").

While the documentation for writing new checks was somewhat lacking, I was able

to convert the existing check and test it rather cleanly https://github.com/django/django/pull/2244

However, this change created some issues I didn't foresee when writing or testing

this through Django's test suite. In particular this warning for un-applied migrations

was triggered when running the tests for a project and when running the migrate command

itself. Obviously if one is running migrate to resolve the problem they don't need

to be told to run migrate. It seemed to me that there would/should be a way to suppress

these checks for these cases and the issue here was a shortcoming of the system

check framework (or my understand of how to use it). Instead my work was reverted 

in favor of restoring the check only in the runserver command. Andrew's comment in 

the ticket seemed to indicate that I was misunderstanding the purpose of the system 

checks and that this check for un-applied migrations did not fit into its purpose.

My questions for this list are:

Should the check for un-applied migrations work within the new system checks framework?

If so, how can we address this issue that in my original implementation these checks

are shown when they are clearly not needed such as when running migrate?

If not, can I get some more clarity on the purpose of these system checks?

Thanks,

Mark

[Andrew Godwin]

So, I don't think it should work within that framework, as having unapplied migrations is not a _system issue_.

It's only really a helpful reminder that fits into the most common part of the workflow (that being that you git pull, new migrations come in, and you run runserver to check out the site and things might start breaking). There's no need for it to run during any other command (in fact, the only one it might make sense with is shell). The checks framework is for whole-project warnings IMO (like "your foreignkeys clash" or "your tests are structured wrong"), and this just isn't that, it's a helpful workflow reminder that we could in fact remove without much problem.

I'm not sure what a good description on the overall purpose of the checks is would be - others might be able to elaborate more - but from a practical point of view, I only want that code to run when you run runserver (much like when you run migrate we *also* run part of makemigrations if there's no migrations to run for another workflow prompt to see if you have changes), so I feel like it's sitting in an alright place for now.

If we could get some context for the checks - things like "this check is only for interactive runserver sessions" - then it would fit there. Perhaps we do, I've not looked into checks too much.

Andrew

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/0cf1d803-7aa2-4342-9f1e-564a560f13ba%40googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Mark Lavin]

I'll agree that it isn't a code issue like models or admin classes which don't validate. However it is a project state problem and is something that users would probably want to know about regardless of whether they are using runserver. There are other workflow issues that could be helped by the checks framework if it fits this case. Uncompiled .po files or using the DB cache without creating the table are two core issues I can think of off the top of my head. There are probably some workflow issues related to static asset pipelining (ala django-compressor or django-pipeline) in the community which could make use of this as well.

As far as not needing this for any other command I don't that's accurate. You would probably want this warning for any command which might interact with the unmigrated tables, shell is one and loaddata is another. If you are missing a migration for the custom User table it could impact createsuperuser or changepassword. Those are just the commands in core/contrib. That doesn't include the numerous commands which exist in the Django eco-system.

Best,

Mark

--
You received this message because you are subscribed to a topic in the Google Groups "Django developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/stZIvqmD85I/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-develop...@googlegroups.com.

To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAFwN1uqmgt302jxMbboqjtcWSco9LORUhHwtVgu6rh46vQS7bw%40mail.gmail.com.