[Django] #31286: Database specific checks always goes to the 'default' backend
Django
-------------------------------------+-------------------------------------
Reporter: Hongtao | Owner: nobody
Ma |
Type: Bug | Status: new
Component: Database | Version: master
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
In an attempt to trigger the check Error mentioned in Tickets: #31144, I
found that the check for database backend doesn't check against the
backend specified, e.g. --database mysql, rather, it always checks against
the 'default' backend.
After diving into the source code, I found the following method defined in
django/db/models/fields/__init__.py
{{{
def _check_backend_specific_checks(self, **kwargs):
app_label = self.model._meta.app_label
for db in connections:
if router.allow_migrate(db, app_label,
model_name=self.model._meta.model_name):
return connections[db].validation.check_field(self,
**kwargs)
return []
}}}
It checks the first db defined in connections rather those provided by
users.
A proposed change would be:
{{{
def _check_backend_specific_checks(self, **kwargs):
app_label = self.model._meta.app_label
errors = []
for db in kwargs['databases'] or ['default']:
if router.allow_migrate(db, app_label,
model_name=self.model._meta.model_name):
errors.extend(connections[db].validation.check_field(self,
**kwargs))
return errors
}}}
It worked as intended on my local machine.
I would happily provide a patch for this one.
--
Ticket URL: <https://code.djangoproject.com/ticket/31286>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Hongtao Ma | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):
* cc: Simon Charette (added)
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Thanks, this was missed in 0b83c8cc4db95812f1e15ca19d78614e94cf38dd. We
should use here the same approach and pass `databases` to these checks:
{{{
diff --git a/django/db/models/fields/__init__.py
b/django/db/models/fields/__init__.py
index 6bd95f542c..c54b8f6e31 100644
--- a/django/db/models/fields/__init__.py
+++ b/django/db/models/fields/__init__.py
@@ -335,11 +335,13 @@ class Field(RegisterLookupMixin):
else:
return []
- def _check_backend_specific_checks(self, **kwargs):
+ def _check_backend_specific_checks(self, databases=None, **kwargs):
app_label = self.model._meta.app_label
- for db in connections:
- if router.allow_migrate(db, app_label,
model_name=self.model._meta.model_name):
- return connections[db].validation.check_field(self,
**kwargs)
+ if databases is None:
+ return []
+ for alias in databases:
+ if router.allow_migrate(alias, app_label,
model_name=self.model._meta.model_name):
+ return connections[alias].validation.check_field(self,
**kwargs)
return []
def _check_validators(self):
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31286#comment:1>
Django
-------------------------------------+-------------------------------------
| Ma
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: nobody => Hongtao Ma
* status: new => assigned
Comment:
Applied the patch, two testcases failed, however:
* FAIL: test_checks_called_on_the_default_database
(check_framework.test_multi_db.TestMultiDBChecks)
* FAIL: test_checks_called_on_the_other_database
(check_framework.test_multi_db.TestMultiDBChecks)
{{{
for alias in databases:
if router.allow_migrate(alias, app_label,
model_name=self.model._meta.model_name):
return connections[alias].validation.check_field(self,
**kwargs)
return []
}}}
The early return will cause only a single backend be checked, in case of
multiple backends provided:
{{{
(djangodev) ➜ my_test_proj git:(iss_check) ✗ python manage.py check
--database sqlite3 --database mysql
System check identified no issues (0 silenced).
(djangodev) ➜ my_test_proj git:(iss_check) ✗ python manage.py check
--database mysql
SystemCheckError: System check identified some issues:
ERRORS:
issue.AnotherIndex.name: (mysql.E001) MySQL does not allow unique
CharFields to have a max_length > 255.
issue.Networkservergroup.groupname: (mysql.E001) MySQL does not allow
unique CharFields to have a max_length > 255.
System check identified 2 issues (0 silenced).
(djangodev) ➜ my_test_proj git:(iss_check) ✗
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31286#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Hongtao Ma | Owner: Hongtao
| Ma
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by felixxm):
You have to pass `databases={'default', 'other'}` to `model.check()` calls
in `TestMultiDBChecks`.
--
Ticket URL: <https://code.djangoproject.com/ticket/31286#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Hongtao Ma | Owner: Hongtao
| Ma
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Hongtao Ma):
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
[https://github.com/django/django/pull/12472 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/31286#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Hongtao Ma | Owner: Hongtao
| Ma
Type: Bug | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
Comment:
Thanks for the patch @Taoup!
--
Ticket URL: <https://code.djangoproject.com/ticket/31286#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Hongtao Ma | Owner: Hongtao
| Ma
Component: Database layer | Version: master
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"271fdab8b78af558238df51c64b4d1c8dd0792bb" 271fdab]:
{{{
#!CommitTicketReference repository=""
revision="271fdab8b78af558238df51c64b4d1c8dd0792bb"
Fixed #31286 -- Made database specific fields checks databases aware.
Follow up to 0b83c8cc4db95812f1e15ca19d78614e94cf38dd.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31286#comment:6>