Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#23749 closed Cleanup/optimization (fixed)

Add an example of using database routers in migrations

Reported by: Alfred Perlstein Owned by: Tim Graham <timograham@…>
Component: Documentation Version: 1.7
Severity: Normal Keywords: migrations dbrouter
Cc: me@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

It seems that the migration system in 1.7 does not properly query the dbrouter setup when a migrations.RunPython method is invoked.

Running the following command:

python manage.py migrate mycompany --noinput --traceback --database=logs --verbosity=2

Never seems to call the router for the following migration, so as you can see I've added code to parse sys.argv as an experiment which seems to sort-of work, but really looks like the wrong way to do this.

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations
from applianceUI.mycompany.dbrouter import LogRouter
import sys

def bkd_default_ports(apps, schema_editor):
    db = None
    for arg in sys.argv:
        if '=' not in arg:
            continue
        print "arg: ",  arg
        k, v = arg.split('=', 2)
        print "k: '%s', v: '%s'  " % (k,v)

        if k == "--database":
            print arg
            db = v
            break

    print "db is %s" % db
    router = LogRouter()
    Blockd = apps.get_model("mycompany", "Blockd")

    if router.allow_migrate(db, Blockd) is False:
        return

    try:
        bkd = Blockd.objects.order_by("-id")[0]
    except IndexError:
        bkd = Blockd.objects.create()
    if bkd.bkd_ports is "":
        bkd.bkd_ports = "80, 8080, 3128"
        bkd.save()


class Migration(migrations.Migration):

    dependencies = [
        ('mycompany', '0005_auto_direction_and_indexes'),
    ]

    operations = [
        migrations.RunPython(bkd_default_ports),
    ]

The arguments apps and schema_editor didn't give me enough data to make my decision. How can i properly derive that the database is "logs"?

Attachments (1)

patch (5.3 KB ) - added by Markus Holtermann 10 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 by Alfred Perlstein, 10 years ago

Traceback I get is as follows:

+ psql -U pgsql -d postgres -c 'create database historicallogs'
CREATE DATABASE
+ python manage.py migrate mycompany --noinput --traceback --database=logs
Operations to perform:
  Apply all migrations: mycompany
Running migrations:
  Applying mycompany.0001_initial... OK
  Applying mycompany.0002_auto_20140904_1549... OK
  Applying mycompany.0003_auto_20141006_1134... OK
  Applying mycompany.0004_blockd_bkd_ports... OK
  Applying mycompany.0005_auto_direction_and_indexes... OK
  Applying mycompany.0006_auto_20141103_1539...Traceback (most recent call last):
  File "manage.py", line 42, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/commands/migrate.py", line 160, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/usr/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 63, in migrate
    self.apply_migration(migration, fake=fake)
  File "/usr/local/lib/python2.7/site-packages/django/db/migrations/executor.py", line 97, in apply_migration
    migration.apply(project_state, schema_editor)
  File "/usr/local/lib/python2.7/site-packages/django/db/migrations/migration.py", line 107, in apply
    operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
  File "/usr/local/lib/python2.7/site-packages/django/db/migrations/operations/special.py", line 117, in database_forwards
    self.code(from_state.render(), schema_editor)
  File "/usr/local/www/applianceUI/applianceUI/mycompany/migrations/0006_auto_20141103_1539.py", line 8, in bkd_default_ports
    bkd = Blockd.objects.config()
  File "/usr/local/www/applianceUI/applianceUI/freeadmin/models/__init__.py", line 59, in config
    obj = self.order_by("-id")[0]
  File "/usr/local/lib/python2.7/site-packages/django/db/models/query.py", line 177, in __getitem__
    return list(qs)[0]
  File "/usr/local/lib/python2.7/site-packages/django/db/models/query.py", line 141, in __iter__
    self._fetch_all()
  File "/usr/local/lib/python2.7/site-packages/django/db/models/query.py", line 966, in _fetch_all
    self._result_cache = list(self.iterator())
  File "/usr/local/lib/python2.7/site-packages/django/db/models/query.py", line 265, in iterator
    for row in compiler.results_iter():
  File "/usr/local/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 700, in results_iter
    for rows in self.execute_sql(MULTI):
  File "/usr/local/lib/python2.7/site-packages/django/db/models/sql/compiler.py", line 786, in execute_sql
    cursor.execute(sql, params)
  File "/usr/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python2.7/site-packages/django/db/utils.py", line 94, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/usr/local/lib/python2.7/site-packages/django/db/backends/utils.py", line 65, in execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python2.7/site-packages/django/db/backends/sqlite3/base.py", line 485, in execute
    return Database.Cursor.execute(self, query, params)
django.db.utils.OperationalError: no such table: mycompany_blockd
+ su - pgsql -c '/usr/local/bin/pg_ctl stop -w -D /data/pgsql/data'
waiting for server to shut down.... done
server stopped
+ exit 1


My dbrouter looks like such:

# Router for DB logs

postgres_models = ('log', 'logrollup', 'ipwhitelist', 'urlwhitelist')
tmplog_models = ('blocklog',)

class LogRouter(object):
    '''
    A router to control all database operations for historical logs.
    '''

    def db_for_read(self, model, **hints):
        '''
        Attempts to read historical logs go to logs db.
        '''
        if model._meta.model_name in postgres_models:
            return 'logs'
        if model._meta.model_name in tmplog_models:
            return 'tmplogs' 
        return None

    def db_for_write(self, model, **hints):
        '''
        Attempts to read historical logs go to logs db.
        '''
        if model._meta.model_name in postgres_models:
            return 'logs'
        if model._meta.model_name in tmplog_models:
            return 'tmplogs'
        return None

    def allow_relation(self, obj1, obj2, **hints):
        # XXX: need cy to revisit this.
        # "apps" is not defined during initial migrations for some
        # reason and this throws a traceback.
        #if obj1._meta.app_label in apps or obj2._meta.app_label in apps:
        #    return True
        return None

    def allow_migrate(self, db, model):
        '''
        Make sure the historical logs only appear in the 'logs'
        database.
        '''
        if db == 'logs':
            return model._meta.model_name in postgres_models
        elif model._meta.model_name in postgres_models:
            return False
        if db == 'tmplogs':
            return model._meta.model_name in tmplog_models
        elif model._meta.model_name in tmplog_models:
            return False

        # otherwise do not allow migrations for non-logs/tmplogs migrations.
        if db == 'logs' or db == 'tmplogs':
            return False
        return None

comment:2 by Tim Graham, 10 years ago

I think this is a duplicate of #23273 (or at least closely related to). Have you read that ticket?

comment:3 by Alfred Perlstein, 10 years ago

To be honest this is more like #22583.

#23273 is too complex for me to parse, there may be some overlap, but I do not think so.

All that is needed is that we somehow pass the --database=FOO option to the RunPython method as a kwarg so that the router can provide a function to query that... OR that the migration itself can decide.

I am not sure why the name of the db is not given to the RunPython method.

This is a relatively simple request, can this please be fixed?

comment:4 by Tim Graham, 10 years ago

Ah yes, that was the ticket I was looking for but couldn't find - forgot it had been closed. Anyway, wouldn't schema_editor.connection.alias give you the name of the database? If so, we should likely document the solution.

comment:5 by Tim Graham, 10 years ago

Component: MigrationsDocumentation
Easy pickings: set
Summary: migrations.RunPython broken with dbrouters.Document how to get the database alias in migrations
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

comment:6 by Bibhas C Debnath, 10 years ago

I just verified that schema_editor.connection.alias does return the current connection in use, even when --database=foobar is used. We could mention this in the data migration doc and/or the `SchemaEditor` doc. SchemaEditor doc could use a Properties section along with the Methods section.

Version 0, edited 10 years ago by Bibhas C Debnath (next)

comment:7 by Bibhas C Debnath, 10 years ago

Cc: me@… added

comment:8 by Tim Graham, 10 years ago

Resolution: worksforme
Status: newclosed

comment:9 by Alfred Perlstein, 10 years ago

Can someone give me a pointer to how to edit the page

https://docs.djangoproject.com/en/1.7/topics/migrations/#data-migrations

-Or-

https://docs.djangoproject.com/en/1.7/ref/schema-editor/

?

To be perfectly honest the addition to the RunPython section (https://github.com/django/django/blob/aa5ef0d4fc67a95ac2a5103810d0c87d8c547bac/docs/ref/migration-operations.txt#L316-L322) is not something that I would have been able to find via google, nor is it likely I would have been able to understand it.

I'm perfectly willing to update the docs for the migration or schema_editor pages, I just need a pointer how to get started. Is it a wiki I can log into? Or is there a git repo I can clone?

Please advise.

thank you!

-Alfred

comment:10 by Tim Graham, 10 years ago

Resolution: worksforme
Status: closednew

comment:11 by Alfred Perlstein, 10 years ago

Owner: changed from nobody to Alfred Perlstein
Status: newassigned

I have created a pull request against documentation here:

https://github.com/django/django/pull/3822

comment:12 by Alfred Perlstein, 10 years ago

If this looks OK I would like to document the connection handle being inside the schema_editor on another page. Please let me know if this document addition is in the correct path and if so I will look into augmenting the schema_editor page.

Is it the norm to document member variables? Is there an existing example? The reason I ask is that schema_editor only has its methods documented and no member variables.

thank you.

comment:13 by Alfred Perlstein, 10 years ago

Tim,

I've submitted a github pull request and tried my best to follow the guide on submitting patches.

I have no spelling errors and no warnings that are in files I have touched.

Can the github pull request be merged at your convenience?

https://github.com/django/django/pull/3822

comment:14 by Alfred Perlstein, 10 years ago

Owner: Alfred Perlstein removed
Status: assignednew

comment:15 by Markus Holtermann, 10 years ago

Has patch: set

comment:16 by Tim Graham, 10 years ago

Patch needs improvement: set

comment:17 by Claude Paroz, 10 years ago

in reply to:  17 ; comment:18 by Markus Holtermann, 10 years ago

Replying to claudep:

#23879 is somewhat related. See this POC branch: https://github.com/wrwrwr/django/compare/feature/always-test-gis-and-postgres

TBH, I don't see any relation to the issue here. It's not about running on a specific vendor, it's about different databases, probably even of the same vendor.

in reply to:  18 comment:19 by Alfred Perlstein, 10 years ago

Replying to MarkusH:

Replying to claudep:

#23879 is somewhat related. See this POC branch: https://github.com/wrwrwr/django/compare/feature/always-test-gis-and-postgres

TBH, I don't see any relation to the issue here. It's not about running on a specific vendor, it's about different databases, probably even of the same vendor.

+1

comment:20 by Markus Holtermann, 10 years ago

Last edited 10 years ago by Markus Holtermann (previous) (diff)

by Markus Holtermann, 10 years ago

Attachment: patch added

comment:21 by Alfred Perlstein, 10 years ago

Which version of the pull request was this made against? It seems to be done against an older version and will not apply cleanly.

comment:22 by Alfred Perlstein, 10 years ago

Oh, it applies, OK. I'll push it shortly thank you.

comment:23 by Alfred Perlstein, 10 years ago

OK, it is pushed, Thank you Markus.

comment:24 by Markus Holtermann, 10 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:25 by Tim Graham, 10 years ago

Summary: Document how to get the database alias in migrationsAdd an example of using database routers in migrations

comment:26 by Tim Graham <timograham@…>, 10 years ago

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In db3f7c15cbf4c9025e83065d1302d0e61d570331:

Fixed #23749 -- Documented how to use the database alias in RunPython.

Thanks Markus Holtermann for review and feedback.

comment:27 by Tim Graham <timograham@…>, 10 years ago

In 01487684123342f052e139291175f4ea3d11a09b:

[1.7.x] Fixed #23749 -- Documented how to use the database alias in RunPython.

Thanks Markus Holtermann for review and feedback.

Backport of db3f7c15cbf4c9025e83065d1302d0e61d570331 from master

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