DB Cache table creation (createcachetable) ignores the DB Router

[zvikico]

When using database-based cache (using django.core.cache.backends.db.DatabaseCache cache backend), one uses the createcachetable command to create the table. This command completely ignores any database routers (in multiple databases situation) which are installed in the settings file. Instead, one can specify a target database with the --database option.

One could argue that this is not a bug, but rather a feature (undocumented one, none the less). However, the DatabaseCache class itself does use the installed router. This creates a confusion: if the router routes to a different database, one may install the table on the wrong database, but will not be able to access it.

I believe the best approach is to follow the syncdb convention: the cache table should not be created when the router returns false form the allow_syncdb method. The table will only be created when the matching DB is specified (or for the default if it is relevant).

[Russell Keith-Magee]

Agreed - allow_syncdb() instructions should be honored. As a side note, there is a related refactoring in the test code (db.backends.creation.create_test_db) -- allow_syncdb is being checked there prior to invoking the database cache table command.

[Aymeric Augustin]

Attached patch resolves the problem described above.

While writing it, I noticed that createcachetable could be much more automatic. Currently the docs at ​http://docs.djangoproject.com/en/dev/topics/cache/#database-caching suggest first creating the table, then adding the cache backend settings. Couldn't we do the opposite: first add the cache backend settings, then create the table? createcachetable would go through the cache backends, find all instances of BaseDatabaseCache, and create the tables with the appropriate name, like this:

from django.conf import settings
from django.core.cache import get_cache

for cache_alias in settings.CACHES:
    cache = get_cache(cache_alias)
    if isinstance(cache, BaseDatabaseCache):
        tablename = cache._table
       ... create the table ...

What do you think? Should I create a different ticket for this?

[Jacob]

Patch looks correct, but it needs tests.

And yeah, I really like the idea of createcachetable inspecting settings.CACHES, but please do take that to another ticket.

[Jacob]

By the way, there are tests for createcachetable in regressiontests/cache/tests.py; we just need a new test method that tests it with the multidb flag. The test suite should already have an "other" database definition for you to test against.

[Aymeric Augustin]

New patch includes a test case.

[Aymeric Augustin]

New patch uses the new @override_settings decorator and remove setUp / tearDown.

[Aymeric Augustin]

jacobkm said on IRC I could mark it as RFC.

[Jannis Leidel]

In [16510]:

Fixed #15255 -- Stopped database cache from ignoring database routers when creating the cache table. Thanks, aaugustin.

[Jannis Leidel]

I rolled back the changes in r16515.

[Aymeric Augustin]

OK, I wasn't using a multi-db capable django.test.TestCase in the v3 patch.

As far as I can tell, two things were still broken when the change was rolled back:

• assertNumQueries missed the using= keyword argument
• we expect two queries, not one

I just uploaded a v4 patch that builds upon jezdez' work by fixing this two issues. The "cache" tests pass on mysql, sqlite and postgresql.

[Aymeric Augustin]

Oops. 15255.4.diff and 15255.4.2.diff are the same file. Trac won't let me remove one.

[Aymeric Augustin]

I just updated the patch against trunk. I checked that the tests pass under SQLite, PostgreSQL, MySQL and Oracle.

[Jacob]

Milestone 1.4 deleted

[Aymeric Augustin]

In [17114]:

Fixed #15255 -- Ensured createcachetable honors database routers.