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

Set default_auto_field to 'AutoField' #2829

Merged
merged 1 commit into from
Sep 9, 2021

Conversation

Flimm
Copy link
Contributor

@Flimm Flimm commented Apr 8, 2021

Django 3.2 introduces DEFAULT_AUTO_FIELD . If you set this to something other than 'django.db.models.AutoField', migrations will need to be generated for allauth's models. This pull request sets default_auto_field just for these apps so that no migrations need to be generated.

It also hides warnings on Django 3.2.

Fixes: #2826

This will avoid warnings in Django 3.2 and will avoid the creation of
new migrations.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 93.581% when pulling 3c1f7fe on Flimm:avoid_auto_field_warnings into 3533862 on pennersr:master.

@czue
Copy link

czue commented Jun 17, 2021

What's the status of this issue? Any plans for a merge/release anytime soon?

@adamchainz
Copy link
Contributor

It would be better to migrate all the models to BigAutoField with explicit declarations. This would prevent them from exhausting ID's, which is the reason Django added DEFAULT_AUTO_FIELD in the first place. But that might be a major version bump.

@czue
Copy link

czue commented Jun 17, 2021

Yeah agreed that makes sense in the long term. In the short term the creation of these migrations can be problematic for new projects, which can easily introduce migrations that depend on files that only exist in a virtualenv on a single dev's machine, and then lead to confusing "missing migration" error when trying to deploy on another environment.

If the release process isn't too painful, do you think it might make sense to roll the fixes out in two stages to avoid people getting tripped up in the interim?

@adamchainz
Copy link
Contributor

Two fixes would be a good idea, yes. (I am not a maintainer, just a user.)

@simkimsia
Copy link
Sponsor Contributor

While waiting for this to be merged, what's a regular user to do to at least temporary suppress the warning without triggering a migration file at least?

@Flimm
Copy link
Contributor Author

Flimm commented Sep 4, 2021

While waiting for this to be merged, what's a regular user to do to at least temporary suppress the warning without triggering a migration file at least?

My post on Stack Overflow explains a workaround. Also, be aware of this bug #2853.

@simkimsia
Copy link
Sponsor Contributor

simkimsia commented Sep 5, 2021

@Flimm

Thank you. HOwever, when I followed your workaround including the one for bug #2853, i get the following

ERRORS:
django_1    | account.EmailAddress.user: (fields.E301) Field defines a relation with the model 'auth.User', which has been swapped out.
django_1    | 	HINT: Update the relation to point at 'settings.AUTH_USER_MODEL'.
django_1    | socialaccount.SocialAccount.user: (fields.E301) Field defines a relation with the model 'auth.User', which has been swapped out.
django_1    | 	HINT: Update the relation to point at 'settings.AUTH_USER_MODEL'

Suggestions?

Update

Forget it. I just realized is similar to the issue raised by @Flimm

So I wrote the workaround in #2946

@brianhelba
Copy link
Contributor

@pennersr This is an important compatibility fix for Django 3.2, and it's generated significant user attention. Could you please review this?

After this is merged, could we also get a PyPI release?

@pennersr pennersr merged commit ea3720f into pennersr:master Sep 9, 2021
@Flimm
Copy link
Contributor Author

Flimm commented Sep 10, 2021

Yay! Thank you for merging.

@brianhelba
Copy link
Contributor

@pennersr Thanks! Could you issue a new PyPI release to include this change?

@Reston
Copy link

Reston commented Sep 30, 2021

I had this problem!

Waiting for this to be released 😄

@pennersr any update?

@Flimm
Copy link
Contributor Author

Flimm commented Feb 17, 2022

The latest release on PyPI does contain the fix, now.

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

Successfully merging this pull request may close these issues.

Django 3.2: multiple warnings
8 participants