MigrationRecorder does not obey db_router allow_migrate rules

[froomzy]

Hi,

We have a multi-db setup. We have one connection that is for the django project, and several connections that talk to other dbs for information (ie models with managed = False). Django should only create tables in the first connection, never in any of the other connections. We have a simple router that does the following:

class Router(object):

    def allow_migrate(self, db, model):
        if db == 'default':
            return True
        return False

Current Behaviour

• We run our functional tests and the migrate command is called against each connection when the test databases are created (see django/test/runner.py, setup_databases, line 300-ish, which calls django/db/backends/creation.py, create_test_db, line 377-ish)
• When this migrate runs, it tries to apply our migrations, which tries to record that a migration has been applied (see django/db/migrations/executor.py, apply_migration, which has several calls to self.recorder.record_applied).
• The first thing that record_applied does is a call to self.ensure_schema() (see django/db/migrations/recorder.py, record_applied, lien 66-ish).
• ensure_schema checks to see if the Migration model is in the tables in the connection. If it does not find the table then it tries to create the table.

I believe that this is incorrect behaviour when a db_router has been provided. If using the router above, my expectation would be that the table is not created on any connection other than the 'default' connection. Looking at the other methods on the MigrationRecorder, I would expect that there will be similar issues with applied_migrations and record_unapplied.

[Josh Smeaton]

I don't think you've implemented your router correctly, but I'd need to check the router code to see if it's called multiple times (num_dbs*num_models) to be sure. This is how we implement our routers for allow_migrate:

def allow_migrate(self, db, model):
        if db == 'other':
            return model._meta.app_label == 'legacy_app' # allow migration for new django models implemented in legacy db
        elif model._meta.app_label == 'legacy_app':  # do not allow migration for legacy on any other db
            return False
        return None # this router not responsible

So, I'm not sure if there is a bug or not (I'll let someone more familiar answer that), but this is what works for us.

[Anssi Kääriäinen]

#22583 is somewhat related. It deals with the inability to skip RunSQL/RunPython operations on given database.

[froomzy]

@jarshwah: I don't think it is the router. Mainly because the router is not called at this point. Which is what I believe is the bug.

@akaariai: I agree that there are similarities. Surely I should be able to manage which connections I actually run migrations against. That seems to be what the db_router is trying to do.

I thought that something like the following would solve our problem:

    from django.db import router
	
	.
	.
	.

    def ensure_schema(self):
        """
        Ensures the table exists and has the correct schema.
        """
        # If the table's there, that's fine - we've never changed its schema
        # in the codebase.
        if self.Migration._meta.db_table in self.connection.introspection.get_table_list(self.connection.cursor()):
            return
        # Make the table
	# Change below, similar to how allowed_to_migrate in django/db/migrations/operations/base.py works
	if router.allow_migrate(self.connection, self.Migration):
		with self.connection.schema_editor() as editor:
			editor.create_model(self.Migration)

But this means that changes to applied_migrations, record_applied, and record_unapplied need to be made, so that it doesn't try to write to a none existent table.

For us this is not an urgent issue, as we have appropriate permissions to those connections that are not our core connection. But I do have a concern for any time that we are using a true read-only connection, where we do not have the CREATE TABLE permission. Because this code, as it exists, will blow up in this situation. I tested that with a read-only connection in our db setup, and got an insufficient permissions error.

Thanks,
Dylan

[Andrew Godwin]

This is an issue, but bear in mind that migrate must be separately executed for each database alias, so this isn't going to happen unless you specifically run migrate on a database that you know isn't supposed to have migrations on it.

I think the best fix would be to have migrations refuse to run entirely and just exit if the migration model is not allowed to be created on the target database.

[froomzy]

I see what you mean about the needing to run migarte for each database. I noticed this with the django test runner, where it is trying to run a migration against every connection alias that we have. So that might be an issue with the test runner as much as with the migrations stuff.

Thanks,
Dylan

[Dominique PERETTI]

Just stumbled on this issue.
With a properly written router, I expected the migrations of each app to be executed in the right database by just using :

manage.py migrate

Could this be the behavior ?

Instead it's assuming I'm using the default database, OK, no, but ok :-)... and it doesn't follow the allow_migrate rule and creates in the default database the tables that are supposed to live exclusively in the another one (NOT OK !).

So for now the workaround for me is to use a shell script where the app and the database are specified.:

./manage.py migrate inapppurchase --database=iap
./manage.py migrate myapp --database=default

[Andrew Godwin]

dperetti: Just like syncdb, migrate only runs on one database at a time, so you must execute it individually for each database, as you suggest. This is by design.

froomzy: Yes, this is likely a test runner issue, that would be doing this kind of migrate-on-everything. I think the suggested fix of refusing to migrate databases where allow_migrate on the migration model returns False will still work, as long as it errors in a way we can catch in the test runner.

[Dominique PERETTI]

Replying to andrewgodwin:

dperetti: Just like syncdb, migrate only runs on one database at a time, so you must execute it individually for each database, as you suggest. This is by design.

The question is : is it the best design ? When I run migrate, I don't want to know about how the router is configured. I just want to migrate the app.
If the router dispatches the tables of an app to different databases, then I want migrate to operate on those.
In other words, I think it would make sense to make migrate database agnostic.

[Dominique PERETTI]

Another side issue : we cannot just manage.py migrate someapp if the someapp is a "django <1.7" app without migration : the old syncdb behavior is not applied when an application is specified.
So if want the old apps to sync, I have to just run manage.py migrate, without argument... which will create unwanted tables when we have multiple databases.

[froomzy]

Hi guys,

Just wondering if there is any chance this will make it into 1.8? Or soon there after?

Thanks,
Dylan

[Tim Graham]

It's not clear to me what fixing this issue involves exactly. Anyway, it doesn't appear anyone is working on it so there's no timetable for its resolution.

[Kevin Christopher Henry]

I wanted to chime in here to broaden the scope of the bug, as I was affected by it recently in a different context.

The bigger issue is not just that the framework tries to create the migrations table where it's not needed, but that it marks as applied migrations that in fact were not. That puts the database in an inconsistent state, at least as far as migrations are concerned. It's a harmless inconsistency for the specific settings file used at that specific moment in time, but it lays the seed for big problems down the line. For example, if you later decide to change your routing scheme, or (as happened in my case) if you have multiple projects with different settings using the same app on the same database.

In terms of a solution, what seems straightforward to my untrained eye is for the migration framework to simply not record migrations as applied that it didn't apply (and as a corollary, it shouldn't try to create the migration table on a database if it doesn't need to record any migrations there). The fix suggested above ("to have migrations refuse to run entirely and just exit if the migration model is not allowed to be created on the target database") doesn't address the broader issue.

[Kevin Christopher Henry]

Let me amend my last comment: I think that having migrate blow up in this situation would in fact solve the problem with having an inconsistent migrations table, which is the most important thing.

My question is, since allow_migrate() operates at the model level and the migrate command operates at the app level, wouldn't this make it impossible to migrate some models of an app but not others?

[Joris]

I can confirm marfire's findings.

1/ For example even with this router applied:

class testerouter(object):
	def allow_migrate(self, db, app_label, model_name=None, **hints):
		return False

And then executing:

migrate --database=replica

All apps and models migrations get applied to this replica database, while according to the router nothing should happen at all.

2/ From the documentation it is not clear whether it is possible to isolate certain models from certain databases, or whether isolation can only be done on the app-level. From the description in the documentation, it seems possible to use the model_name argument for this, but by the way django stores its migrations, I don't see how that could work.

[Ryan Vennell]

Just to further confirm: I've been wrestling with this problem for a couple days. I have multiple databases and multiple apps. When I'm running tests, the router allow_migrate works properly to control which migrations can be applied to which database/app.

The actual migration portion works fine, but when it comes time to actually write the migration history, it seems to only have a concept of the default database. When it runs ensure_schema, it's expecting that tables should exist on the default DB when they really only exist for certain other apps/databases. This leads to a ProgrammingError where it can't find tables that it shouldn't be checking for in the first place.

[Daniel van Flymen]

Can you please have a look at ​http://stackoverflow.com/questions/40893868/using-redshift-as-an-additional-django-database?noredirect=1#comment69004673_40893868, which provides a decent use case for this bug.

[Jacob Walls]

Since 3.1 you can set 'TEST': {'MIGRATE': False} to avoid running migrations on a given db connection, so that solves the test runner issue.

Still, even if you do that, apps are still synced (see fix to #32012), Django ends up calling migrate to do the syncing, and this will cause queries in MigrationRecorder.ensure_schema() that might create tables (or fail with permission errors, see #27141).

I plan to open a PR to do roughly this from comment:13:
it shouldn't try to create the migration table on a database if it doesn't need to record any migrations there

[Jacob Walls]

​PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In 92412aa:

Fixed #23273 -- Avoided creation of django_migrations table when there are no migrations to apply.