[Django] #21755: db.utils _route_db can raise AttributeError: 'NoneType' object has no attribute '_state'
Django
attribute '_state'
----------------------------------------------+--------------------
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
----------------------------------------------+--------------------
If `instance=None` is passed to `_route_db` in db.utils, then an exception
like the following is raised:
{{{
...
File ".../django/core/management/base.py", line 285, in execute
output = self.handle(*args, **options)
File ".../django/contrib/auth/management/commands/createsuperuser.py",
line 96, in handle
username = self.username_field.clean(raw_value, None)
File ".../django/db/models/fields/__init__.py", line 255, in clean
self.validate(value, model_instance)
File ".../django/db/models/fields/related.py", line 1199, in validate
using = router.db_for_read(model_instance.__class__,
instance=model_instance)
File ".../django/db/utils.py", line 250, in _route_db
return hints['instance']._state.db or DEFAULT_DB_ALIAS
AttributeError: 'NoneType' object has no attribute '_state'
}}}
The code is
[https://github.com/django/django/blob/856aaaf2b15228bd5babbe44654c3c1ca39e8fd7/django/db/utils.py#L243
here]:
{{{
try:
return hints['instance']._state.db or DEFAULT_DB_ALIAS
except KeyError:
return DEFAULT_DB_ALIAS
}}}
It would be better if the None case were handled in the same way that
KeyError is being caught.
--
Ticket URL: <https://code.djangoproject.com/ticket/21755>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
attribute '_state'
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by cjerdonek):
* cc: chris.jerdonek@… (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:1>
Django
attribute '_state'
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by bmispelon):
Hi,
It's not clear to me whether you've managed to trigger this error while
using the ORM or if you have a use-case where you'd like to be able to
pass `instance=None` as a hint to the db router.
If it's the former, do you have a bit of code that reproduces the isssue
and if it's the latter, could you explain why you need to pass
`instance=None` rather than not passing an instance at all?
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:2>
Django
attribute '_state'
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by cjerdonek):
Hi, I've been meaning to elaborate on this because I'm not sure if this
should be changed as I've described.
This error occurred when setting the `USERNAME_FIELD` to a foreign key
field and running the `createsuperuser` command. The base field class's
`validate()` method
[https://github.com/django/django/blob/9ddc358e8d598f38d8443be708962d8b1f34b7e2/django/db/models/fields/__init__.py#L442
doesn't use] the `model_instance` argument, but the `ForeignKey` field
version's
[https://github.com/django/django/blob/9ddc358e8d598f38d8443be708962d8b1f34b7e2/django/db/models/fields/related.py#L1616
does].
Obviously, I needed to change the `createsuperuser` command in other ways
to get `createsuperuser` to work in my use case above, but this error I
encountered in the process seemed unnecessarily obscure.
The possible resolutions of this issue I can think of are:
* pass a valid `model_instance` instead of `None` in the default
implementation of `createsuperuser`, even though it is not needed in the
out-of-the-box case,
* allow `None` to be passed in the foreign key field version of
`validate()` (for example by catching the AttributeError in `db.utils
_route_db()` as I described in my original suggestion),
* raising a simpler-to-decipher error earlier in the process (e.g. by
raising in `createsuperuser` or elsewhere that foreign key fields aren't
allowed), or
* leaving things as is.
Thanks for considering this.
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:3>
Django
attribute '_state'
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by cjerdonek):
By the way, this ticket isn't the place to resolve the following issue in
general, but my preference would be for Django to make it easier to
support the following two use cases out of the box: (1) `USERNAME_FIELD`
being a foreign key, and (2) `USERNAME_FIELD` being a tuple of fields. I
may open tickets for these two issues at some point if they haven't
already been suggested. If the resolution of this ticket can reduce one
roadblock towards use case (1), that would be good. The current issue is
simply one of the first places where things went wrong.
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:4>
Django
attribute '_state'
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by bmispelon):
* easy: 1 => 0
* stage: Unreviewed => Accepted
Comment:
OK, I'm starting to understand a bit better (thanks for clarifying).
I couldn't find any official documentation for `models.Field.validate()`
and in particular, I don't think we've documented what's acceptable to
pass as `model_instance` or not.
Typically, `Field.validate` is called as part of model validation
(`Model.clean_fields`) which always passes a valid `model_instance`) so
there's no issue. In this case though, the problem arises because
`createsuperuser` calls field validation directly, in which case it
doesn't make much sense to pass a `model_instance`.
I think your option 2 is a valid approach, though I would use something
like this rather than catching `AttributeError` (I feel that it's not
specific enough):
{{{#!python
instance = hints.get('instance')
if instance is not None and instance._state.db:
return instance._state.db
return DEFAULT_DB_ALIAS
}}}
Regarding using a `ForeignKey` as a `USERNAME_FIELD`, while it doesn't
seem like a very common use-case, it's not explicitly forbidden by the
documentation [1] which only requires the field to be unique.
I suggest opening a ticket regarding this feature so that we can either
support it or at least document that it's not possible.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:5>
Django
attribute '_state'
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by cjerdonek):
I suggest opening a ticket regarding this feature so that we can either
support it or at least document that it's not possible.
Thanks for your thoughts. Okay, I created #21832 for this.
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:6>
Django
attribute '_state'
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by davidc):
I am running into the same problem for a foreign key in the
`REQUIRED_FIELDS` of my custom user model. It does not have to be the
`USERNAME_FIELD`. I think this would be much more common. In my case I
need all users to have a department.
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:7>
Django
attribute '_state'
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by cjerdonek):
@davidc, for future reference, can you include the stack trace you are
getting? The stack trace I included in my original post references the
username field (in the line, `username =
self.username_field.clean(raw_value, None)`), so it looks like there is
another place in Django's code base that is passing `None` as the model
instance.
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:8>
Django
attribute '_state'
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by dcreech):
Here is the stack trace:
{{{
...
File "C:\Python27\lib\site-packages\django\core\management\base.py",
line 285, in execute
output = self.handle(*args, **options)
File "C:\Python27\lib\site-
packages\django\contrib\auth\management\commands\createsuperuser.py", line
116, in handle
user_data[field_name] = field.clean(raw_value, None)
File "C:\Python27\lib\site-
packages\django\db\models\fields\__init__.py", line 255, in clean
self.validate(value, model_instance)
File "C:\Python27\lib\site-packages\django\db\models\fields\related.py",
line 1199, in validate
using = router.db_for_read(model_instance.__class__,
instance=model_instance)
File "C:\Python27\lib\site-packages\django\db\utils.py", line 250, in
_route_db
return hints['instance']._state.db or DEFAULT_DB_ALIAS
AttributeError: 'NoneType' object has no attribute '_state'
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:9>
Django
attribute '_state'
Reporter: cjerdonek | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anubhav9042):
Replying to [comment:7 davidc]:
> I am running into the same problem for a foreign key in the
`REQUIRED_FIELDS` of my custom user model. It does not have to be the
`USERNAME_FIELD`. I think this would be much more common. In my case I
need all users to have a department.
I think we cannot use '''ForeignKey''' here.
See:[https://docs.djangoproject.com/en/1.6/topics/auth/customizing/#django.contrib.auth.models.CustomUser.REQUIRED_FIELDS]
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: cjerdonek | Owner: nobody
Type: New feature | Status: new
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: timo (added)
* type: Bug => New feature
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:11>
Django
Reporter: cjerdonek | Owner: nobody
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Changes (by anubhav9042):
* cc: anubhav9042@… (added)
* needs_docs: 0 => 1
* has_patch: 0 => 1
* version: 1.6 => master
* component: Database layer (models, ORM) => contrib.auth
Comment:
https://github.com/django/django/pull/2866
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:12>
Django
------------------------------+------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Comment (by anubhav9042):
For now, I have skipped the call to `clean` in case of FK and left the
validation part of `Descriptors`.
I tried to implement what @bmispelon commented, but there seemed to a be
problem with that. In `validate()` in `related.py`, after we get the
correct qs, qs.exists() somehow retriggers call to `validate()` and we get
empty qs and error is raised. Can anyone explain this?
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:13>
Django
------------------------------+------------------------------------
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:14>
Django
------------------------------+------------------------------------
Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"9bc2d766a0c31701a4c0e20a8b04164bb22cf6d5"]:
{{{
#!CommitTicketReference repository=""
revision="9bc2d766a0c31701a4c0e20a8b04164bb22cf6d5"
Fixed #21755 -- Added ForeignKey support to REQUIRED_FIELDS.
This allows specifying ForeignKeys in REQUIRED_FIELDS when using a
custom User model.
Thanks cjerdonek and bmispelon for suggestion and timgraham for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:15>
Django
Reporter: Chris Jerdonek | Owner: nobody
Type: New feature | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"4ff500f2948bfc332b3f4159021cad06e91943d3" 4ff500f2]:
{{{
#!CommitTicketReference repository=""
revision="4ff500f2948bfc332b3f4159021cad06e91943d3"
Refs #21755 -- Fixed createsuperuser crash for required foreign keys
passed in options in interactive mode.
Co-authored-by: Mariusz Felisiak <felisiak...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:16>
Django
--------------------------------+------------------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: New feature | Status: closed
Component: contrib.auth | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b55df4c74a725a1df4efbce8163f995cb0bcd875" b55df4c7]:
{{{
#!CommitTicketReference repository=""
revision="b55df4c74a725a1df4efbce8163f995cb0bcd875"
[4.0.x] Refs #21755 -- Fixed createsuperuser crash for required foreign
keys passed in options in interactive mode.
Co-authored-by: Mariusz Felisiak <felisiak...@gmail.com>
Backport of 4ff500f2948bfc332b3f4159021cad06e91943d3 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21755#comment:17>