System checks don't respect database routers

[Ion Scerbatiuc]

We have the following database set-up:

• the default database is MySQL
• two additional databases are PostGIS

The system check framework (either manage.py check or manage.py runserver) is using the default database (mysql) to validate django.contrib.gis.db.models.Model-based models, and the commands are blowing up with the following error:
AttributeError: 'DatabaseOperations' object has no attribute 'geo_db_type'.

We set-up database routers for the PostGIS models to use the proper connection, but the check framework seems to ignore them.

You can find the detailed stack trace here: ​https://gist.github.com/delinhabit/587d6f26afb4bbc559cb

The discussion that led to this ticket:
​https://groups.google.com/forum/#!msg/django-developers/_QUT4hV43YE/b-1eTcv_5yUJ

I can try to create a PR, though I'm not sure where to start. Specifically my questions are:

• What tests case file should I extend with a regression tests? The tests/multiple_database doesn't test any system checks at all. tests/system_check could be a candidate but I need a multiple database set-up and it doesn't feel right to change that test app. Should I better create a new test app for this use case?
• I think Field._check_backend_specific_checks should be the place, only that at that point it already uses connection and I'm not sure how can I detect what connection should I use from the field instance (or the associated model). Should I run the Database routing code to get the connection alias? (seems a little gross to me)

Any feedback appreciated.

Thanks,
Ion

[Tim Graham]

I think the code for _check_backend_specific_checks() would be roughly like this:

from django.db import connections
for db in connections:
   if allow_migrate(...):
      db.validation.check_field(self, **kwargs)

Let's get some working code and then we can talk about how to test this. I guess it might involve mocking connections to have two different databases as we don't run the tests against two different databases on CI and getting that to work might not be feasible.

[Tim Graham]

If it works, great! For a test, I would create a new file test/check_framework/test_multi_db.py and write a custom database router based on the default/other databases aliases so that the checks are only run on one of the databases and then use mock to verify which connection's validation methods are called. Let me know if you run into trouble. Not positive that will work, but that would be the direction I'd try.

[Ion Scerbatiuc]

The patch was created against master.

If we decide to also include a fix in the next 1.7.x bugfix release, I think it won't work as it is, because of the signature change of the allow_migrate method.

[Tim Graham]

Are you able to open a pull request? That makes review easier.

[Ion Scerbatiuc]

Sure. Here it is: ​https://github.com/django/django/pull/5084

[Tim Graham]

Great. By the way, don't forget to check "Has patch" so the ticket appears in the review queue.

[Tim Graham]

Some test failures need to be addressed.

[Ion Scerbatiuc]

Fix the issue and add regression tests

[Tim Graham]

FYI, there's no need to submit a pull request *and* attach the patch to the ticket.

However, please do uncheck "Patch needs improvement" after updating the pull request so the ticket appears in the review queue.

[Ion Scerbatiuc]

Oh, didn't know that. It's definitely easier to work with pull requests instead of patches.

Thanks!

[Tim Graham <timograham@…>]

In 0cc059c:

Fixed #25172 -- Fixed check framework to work with multiple databases.