Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-add conditional abstract setting to Token model #3858

Closed
adam-thomas opened this issue Jan 21, 2016 · 14 comments
Closed

Re-add conditional abstract setting to Token model #3858

adam-thomas opened this issue Jan 21, 2016 · 14 comments
Milestone

Comments

@adam-thomas
Copy link

adam-thomas commented Jan 21, 2016

When attempting to use a custom model instead of DRF's built-in Token, Django tries very hard to include the Token model even if rest_framework.authtoken isn't in INSTALLED_APPS. Attempting to run migrations on a fresh database will explode with a missing user model error, and an already-set-up database will have a Token table added to it (which also means the user model gets an unwanted auth_token OneToOne field on it).

I believe a simple fix for this behaviour is to reinstate the Meta abstract setting in the Token model. I spent a day or so digging for other ways to avoid using Token in the project I was working on and couldn't find anything, but there may also be other fixes.

@xordoquy
Copy link
Collaborator

xordoquy commented Jan 21, 2016

Should be fixed by #3785

@adam-thomas
Copy link
Author

adam-thomas commented Jan 21, 2016

Unfortunately, that's the exact commit that introduces the breakage. The delayed import doesn't seem to help. I haven't been able to figure out why.

@adam-thomas
Copy link
Author

adam-thomas commented Jan 21, 2016

When I was experimenting and trying to figure out how to work around this, I even deleted the entire TokenAuthentication class, and the problem persisted. I think the Token model is being picked up through other means.

@adam-thomas
Copy link
Author

adam-thomas commented Jan 21, 2016

(We're on Django 1.8.)

@xordoquy
Copy link
Collaborator

xordoquy commented Jan 21, 2016

Do you have some sample project to reproduce the issue ?

@adam-thomas
Copy link
Author

adam-thomas commented Jan 21, 2016

Not at the moment (it's a private project). I'll try to get some time to make one. I imagine it would involve:

  • rest_framework in INSTALLED_APPS (but not rest_framework.authtoken)
  • A User model exists and AUTH_USER_MODEL is set
  • A test calls _meta.get_all_field_names() on that user model

I don't know if the migration ordering issue will occur for the test project - it depends on whether Token is migrated before User. If it does, trying to run the tests will blow up with the SQL error about a missing relation. If that error doesn't appear, the test should show that auth_token is a field on the User model.

@xordoquy
Copy link
Collaborator

xordoquy commented Jan 21, 2016

Now that I read this again, I think I get the issue and indeed the Token should be left abstract if the app isn't in the INSTALLED_APPS.

@xordoquy xordoquy added this to the 3.3.3 Release milestone Jan 21, 2016
@adam-thomas
Copy link
Author

adam-thomas commented Jan 21, 2016

👍 Thanks :)

@jpadilla
Copy link
Member

jpadilla commented Jan 21, 2016

@xordoquy so we keep #3785 and just revert the changes to the Meta class on the model?

@xordoquy
Copy link
Collaborator

xordoquy commented Jan 21, 2016

Yup, that's #3860

@xordoquy
Copy link
Collaborator

xordoquy commented Jan 21, 2016

@adam-thomas thanks for pointing this to our attention.

@adam-thomas
Copy link
Author

adam-thomas commented Jan 21, 2016

Thank you @xordoquy :)

@adam-thomas
Copy link
Author

adam-thomas commented Jan 21, 2016

And, most welcome! Glad to help!

@tomchristie
Copy link
Member

tomchristie commented Jan 21, 2016

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants