rest_framework 3.10.0 - crash with runserver

[cedricfarinazzo]

Checklist

• I have verified that that issue exists against the master branch of Django REST framework.
• I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• This is not a usage question. (Those should be directed to the discussion group instead.)
• This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
• I have reduced the issue to the simplest possible case.
• I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

• django 2.2
• update rest_framework to v3.10.0 with pip (last version in my computer: 3.9.4)
• settings.py

INSTALLED_APPS = [
    ....
    'rest_framework',
    'rest_framework.authtoken',
     ...
]

• python manage.py runserver 0.0.0.0:8080

Expected behavior

Not crash

Actual behavior

user@pc:~/path/to/project/$ python manage.py runserver 0.0.0.0:8080
Watching for file changes with StatReloader
Performing system checks...

Exception in thread Thread-1:
Traceback (most recent call last):
  File "/usr/lib/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.6/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "/home/user/.local/lib/python3.6/site-packages/django/utils/autoreload.py", line 54, in wrapper
    fn(*args, **kwargs)
  File "/home/user/.local/lib/python3.6/site-packages/django/core/management/commands/runserver.py", line 117, in inner_run
    self.check(display_num_errors=True)
  File "/home/user/.local/lib/python3.6/site-packages/django/core/management/base.py", line 436, in check
    raise SystemCheckError(msg)
django.core.management.base.SystemCheckError: SystemCheckError: System check identified some issues:

ERRORS:
<class 'rest_framework.authtoken.admin.TokenAdmin'>: (admin.E040) UserAdmin must define "search_fields", because it's referenced by TokenAdmin.autocomplete_fields.

System check identified 1 issue (0 silenced).

[cedricfarinazzo]

Due to this pull request #6762 ...

[carltongibson]

UserAdmin is Django’s right? In which case we may need to revert...

[carltongibson]

Looks like it does define search_fields.

https://github.com/django/django/blob/cf79f92abee2ff5fd4fdcc1a124129a9773805b8/django/contrib/auth/admin.py#L63

So are you using a custom user model and admin? If so, does adding search_fields like the error says fix it?

[cedricfarinazzo]

Yes I use custom user model and custom admin model.
If I add search_fields = ("email",), it works.

But if I have to do it, it means that it's not backward compatible ...

[tomchristie]

I don’t know if there’s any great options here. I guess we could do a runtime check for if search_fields is set on the user model, and only include it in the autocomplete list if it is?

[carltongibson]

A small break, with an easy fix, between a minor version, especially that is caught by a system check is OK.

The SemVer purists will scream but that's how we've always done it on DRF.

BUT we should add a small clarification to the release notes. (If there's not one already.)

[dalvtor]

Just to provide further information. If a custom admin site is used and the User model is not registered, the following error will occur:

ERRORS:
<class 'rest_framework.authtoken.admin.TokenAdmin'>: (admin.E039) An admin for model "User" has to be registered to be referenced by TokenAdmin.autocomplete_fields.

I fixed it by registering my User model.

[carltongibson]

Yes, that would be expected. TBH I think the system check does the best job here of explaining what’s needed. (We’d just he repeating the Django docs to try to cover it.)

[loganknecht]

Just chiming in on this. It's breaking my stuff as well.

Except, I'm not even doing runserver I'm just doing migrations.

The issue appears to resolve itself if I comment out a line from this file:
venv/lib/python3.7/site-packages/rest_framework/authtoken/admin.py

from django.contrib import admin

from rest_framework.authtoken.models import Token


class TokenAdmin(admin.ModelAdmin):
    list_display = ('key', 'user', 'created')
    fields = ('user',)
    ordering = ('-created',)
    autocomplete_fields = ('user',)


# Commenting this out makes it work
# admin.site.register(Token, TokenAdmin)

Kind of a bummer this happened. I do not understand what this is at all and why it's failing.

[rpkilby]

Hi @loganknecht - any management command (runserver, migrate, etc..) should fail since this is causing a system check failure. You need to do either one of two things:

• Unregister the Token model admin.site.unregister(Token)
• Add search_fields to your customer user admin

[loganknecht]

@rpkilby Thank you for taking the time to respond.

I think my point of confusion for this is why does version 3.9.4 work completely fine, but version 3.10.0 break everything entirely?

In addition to that, why do I now have to do either one of those two things? That doesn't make sense why I have to unregister something I never registered before, or add a field I never used before?

[rpkilby]

I think my point of confusion for this is why does version 3.9.4 work completely fine, but version 3.10.0 break everything entirely?

DRF doesn't use strict semantic versioning. The versioning scheme is slightly different, and the jump from 3.9.4 to 3.10 is considered a "medium" version release, which may contain breaking API changes. You can think of 3.9 and 3.10 as analogous to Django's "feature" releases.

In addition to that, why do I now have to do either one of those two things? That doesn't make sense why I have to unregister something I never registered before, or add a field I never used before?

You've likely enabled it unknowingly. If you're getting this error, then rest_framework.authtoken should be in your INSTALLED_APPS, and the Django admin's autodiscovery should have picked it up. If that's the case, and you don't need the token admin, one option would be to unregister it. This is effectively what you did above by commenting out the register call. If you do want to keep the token admin, then you need to add search_fields to your custom user model's admin.

[loganknecht]

@rpkilby Again, Thank you for taking the time to respond.

Is there somewhere I can read about search_fields configuration and why it needs to be added/what it does?

[rpkilby]

Again, Thank you for taking the time to respond.

No problem - this is useful feedback, as other users are likely to run into the same issue.

Is there somewhere I can read about search_fields configuration

The django admin docs explain autocomplete_fields, and why the related model's admin needs to have search_fields set. In this case, the TokenAdmin added autocomplete_fields = ['user'], and your custom user admin does not have search_fields, which this feature depends on.

[loganknecht]

@rpkilby I think I it's really hard for me to understand what you've explained. I've read the documentation and I kind of understand what autocomplete_fields are used for, and that it is helped by using the search_fields

Does Django Rest Framework have to add the user to its autocomplete_fields? Was that one of that changes from 3.9.4 to 3.10? Is that required?

I think I understand the problem, but I don't have an admin configuration for my django project. What should I do to fix this? Does this mean I simply add:

class User(AbstractBaseUser, PermissionsMixin):
    ...
    search_fields = []
    ...

I would like to politely express some frustration regarding this because it seems like I have to go back and account for changes because of DRF after the fact. And it would be nice to have a clear explanation in the form of documentation from DRF to understand why this information is needed for the DRF admin configurations, and why I have to change my user model to match that.

Is there no way to opt-in to this? I do need to use the rest_framework.authtoken app to support my token authorization, but it just seems weird to have to do this now, because I'm not even making use of the django admin functionality. It also seems weird to have to unregister something. If that's the case, wouldn't it make more sense for me to register the DRF token admin configuration myself as a form of opt-in?

Additionally, for me this isn't a huge deal because my user model hasn't been formalized yet, but I imagine this is going to affect a lot of people who don't have the luxury of easily modifying their custom user model? Is this going to affect them as well? I have been under the impression that having to make changes to the user model after the fact is a hard thing to do.

Thank you for all the work, and the responses. I really appreciate everything.

[cedricfarinazzo]

I think this little change is doing more harm than good. Maybe it's better to revert it ... or to change something to do not force users to do something weird. Updating the doc is the simplest solution but the problem is still here.

And please be careful before merging pull request, a lot of people use this awesome package and their works can be broken with thing like this.

Thanks for all.

[carltongibson]

I'm not even making use of the django admin functionality

But the system check shouldn't fire is django.contrib.admin is not in INSTALLED_APPS.

The usability benefits of autocomplete fields are worth the costs here. Anyone using the admin with a decent number of user records will benefit.

The system check error messages are pretty clear. #6811 adds an extra explanatory note but the admin docs are the canonical source. This isn't a big enough issue to revert over.

[rpkilby]

Does Django Rest Framework have to add the user to its autocomplete_fields?

No, but it is beneficial when there are a large number of users in your database. Otherwise the token admin becomes unusable (rendering a select dropdown for 150k users is slow).

I think I understand the problem, but I don't have an admin configuration for my django project.

If you're not using the Django admin, you could also fix this by just removing django.contrib.admin from the INSTALLED_APPS setting.

I would like to politely express some frustration regarding this because it seems like I have to go back and account for changes because of DRF after the fact.

Of course. There are a few issues that intersect, and the result in this case is surprising/confusing.

Is there no way to opt-in to this? ... It also seems weird to have to unregister something. If that's the case, wouldn't it make more sense for me to register it myself as a form of opt-in?

The admin works via autodiscovery, and 3rd party apps generally self-register on discovery. It would be atypical for an app to provide admin classes and require projects to register them separately. Note that you're effectively opting in by having the 'django.contrib.auth' in your INSTALLED_APPS. If you're not using the admin, then it should be removed from the apps list.

I imagine this is going to affect a lot of people who don't have the luxury of easily modifying their custom user model?

This doesn't require changing the user model - just the admin for the custom user model.