Code

Opened 3 months ago

Last modified 7 weeks ago

#21755 new Bug

db.utils _route_db can raise AttributeError: 'NoneType' object has no attribute '_state'

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

Description

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 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.

Attachments (0)

Change History (10)

comment:1 Changed 3 months ago by cjerdonek

  • Cc chris.jerdonek@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 months ago 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.

comment:3 Changed 3 months ago 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 doesn't use the model_instance argument, but the ForeignKey field version's 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.

comment:4 Changed 3 months ago 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.

comment:5 Changed 3 months ago by bmispelon

  • Easy pickings unset
  • Triage Stage changed from Unreviewed to Accepted

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):

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.

[1] https://docs.djangoproject.com/en/1.6/topics/auth/customizing/#django.contrib.auth.models.CustomUser.USERNAME_FIELD

comment:6 Changed 3 months ago 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.

comment:7 follow-up: Changed 3 months ago 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.

comment:8 Changed 3 months ago 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.

comment:9 Changed 3 months ago by dcreech

@cjerdonek, 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'

Last edited 3 months ago by dcreech (previous) (diff)

comment:10 in reply to: ↑ 7 Changed 7 weeks ago by anubhav9042

Replying to 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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.