Opened 5 years ago

Last modified 2 years ago

#14287 new Bug

TEST_MIRROR is not respected in routers

Reported by: dcramer Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2
Severity: Normal Keywords:
Cc: roman@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When using TEST_MIRROR to proxy database connections in the test environment, the alias is not saved when calling syncdb (and possible other) routines. This causes a problem when you're using routers and checking db aliases.

Here's a quick example:

    def allow_syncdb(self, db, model):
        # analytics has TEST_MIRROR = 'default', so in tests, db is set to 'default' here
        if model._meta.app_label == 'analytics' and db == 'analytics':
            return True

Change History (9)

comment:1 Changed 5 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to worksforme
  • Status changed from new to closed

If TEST_MIRROR is enabled on a database, then syncdb won't be called on that database. That's part of the design contract for TEST_MIRROR.

The analytics alias will still exist, it just acts as a synonym for a connection to the default alias.

I don't see the problem that is being reported here, so I'm marking this worksforme. Feel free to reopen if you can provide a more specific example or explanation of what is (or isn't) happening.

comment:2 Changed 5 years ago by dcramer

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I should try to explain this better, so let's try another example.

We have a "cluster foo" and "cluster bar" databases. In development and tests, we map these to the same cluster, so:

DATABASES = {
    'foo': {},
    'bar': {
        'TEST_MIRROR': 'bar',
    }
}

Now in real conditions, we say we want all apps in foo module to go to foo, and bar module to go to bar, so we setup a router:

class TestMirrorRouter(object):
    def db_for_write(self, model, **hints):
        return model._meta.app_label
    
    db_for_read = db_for_write
    
    def allow_syncdb(self, db, model):
        return db == model._meta.app_label

Now the problem exists, that all queries get mapped to the "bar" alias from the "bar" module, but when syncdb happens it passes the db alias as "foo" instead of "bar" (simply because this is the *actual* connection its happening on). Now our future tests which run simple queries against Bar fail in that the tables were never created on the "foo" database.

comment:3 Changed 5 years ago by dcramer

I should also note, that this may not *always* be the desired behavior. You may also want to not mirror the alias and just run tests under a second database. We tend to use this for simplified local testing (and have even modified the TEST_MIRROR to proxy connections a bit more thoroughly).

Our simple fix for this was in dev to include a TestRouter which just returns True on allow_syncdb for any model, but that's very hacky and makes it hard to test some real behavior.

comment:4 Changed 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

Ok - after speaking with David on IRC, I now understand the problem better.

The important point here is the router behavior is using the app_label as the basis for synchronization. In production, there are two databases, sharded by app. However, in testing, for the sake of simplicity, they're not sharded; they're all on the same database, and TEST_MIRROR is used to make this happen.

However, when syncdb is called on 'foo', none of the 'bar' models are created, because the router will prevent anything not in 'foo' from being synchronized.

This is a slight deviation from the original intent of TEST_MIRROR, so the solution may not lie with fixing TEST_MIRROR. Another approach would be to invoke a mock syncdb for 'bar' on 'foo'. It may even be sufficient (although slightly innefficient) to call syncdb on every database, including the aliased mirrors

comment:5 Changed 4 years ago by t0ster

  • Cc roman@… added

comment:6 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to Bug

comment:7 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:8 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:9 Changed 2 years ago by aaugustin

  • Status changed from reopened to new
Note: See TracTickets for help on using tickets.
Back to Top