Opened 10 years ago

Closed 9 years ago

#22583 closed New feature (fixed)

Allow per-database migrations in multidb configurations

Reported by: Anssi Kääriäinen Owned by: nobody
Component: Migrations Version: dev
Severity: Normal Keywords:
Cc: Loic Bistuer 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

Currently a database router can be used to skip operations on given model. However, this approach doesn't work at all for RunPython or RunSQL commands - especially RunSQL has no notion of which models the SQL touches. For this reason there should be some way to specify which databases the migration should apply to. The simplest way seems to be addition of using kwarg to RunPython and RunSQL commands. For example:

operations = [
    RunSQL("create table foobar(id serial primary key);", using='default'),
    RunSQL("create table foobar(id integer primary key auto_increment) engine myisam;",
           using=('mysql_alias1', 'mysql_alias2'))
]

However the problem here is that this assumes the migrations know the used database aliases. This doesn't work for 3rd party app migrations. For these cases "for_vendor='mysql'/'postgresql'/..." argument would work better.

Still, there might be need to allow skipping whole apps for migations - say you have a 3rd party app you don't want to migrate on database other - my understanding is that one can only skip all model operations on given database, but there is no way to force skip of RunSQL and RunPython commands, too. If it would be possible to define that the app in whole shouldn't be migrated on database other, then there would be no problem when running manage.py migrate.

Change History (8)

comment:1 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted
Type: BugNew feature

comment:2 by Andrew Godwin, 10 years ago

So, it's worth pointing out that RunPython can just switch on the value of schema_editor.connection if it wants to make decisions about the current database, so I'm not sure if I want to add extra logic to RunSQL? Especially a run-per-backend thing...

comment:3 by Andrew Godwin, 10 years ago

Resolution: wontfix
Status: newclosed

I'm going to WONTFIX this and suggest that people use RunPython instead to do any custom logic; they can either call RunSQL from inside that or, more easily, just use schema_editor.execute() if they wish to run raw SQL.

comment:4 by Diego Lorden, 10 years ago

I had this problem too and RunPython was not reliable enough. I created a package to run raw SQL on a specific connection similar to the example given in the ticket description: django-migrations-plus. Contributions are appreciated!

comment:5 by Loic Bistuer, 9 years ago

Has patch: set
Resolution: wontfix
Status: closednew

Reopening after discussing this on multiple occasions on IRC.

RunPython and RunSQL should query the database router like every other operations and hints can be used to assist the router in taking a decision. This is consistent with how db routing is performed in the other parts of the system (i.e. db_for_read and db_for_write):

class MyRouter(object):
    def allow_migrate(self, db, model, **hints):
        if 'target_db' in hints:
            return db == hints['target_db']
        return True

RunPython(forwards, hints={'target_db': 'default'})

This also allows completely opting out of migrations for any given db with:

class MyRouter(object):
    def allow_migrate(self, db, model, **hints):
        return db != 'excluded_db'

PR https://github.com/django/django/pull/3849

comment:6 by Loic Bistuer, 9 years ago

Cc: Loic Bistuer added

comment:7 by Markus Holtermann, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:8 by Loic Bistuer <loic.bistuer@…>, 9 years ago

Resolution: fixed
Status: newclosed

In 8f4877c89d0f28e289399fbb46357623a49e7eb0:

Fixed #22583 -- Allowed RunPython and RunSQL to provide hints to the db router.

Thanks Markus Holtermann and Tim Graham for the review.

Note: See TracTickets for help on using tickets.
Back to Top