-
Notifications
You must be signed in to change notification settings - Fork 531
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
Fix bug 1704038: Upgrade to Django 3.2 #1921
Conversation
I don't understand the CI Docker build failure, locally I can build the Docker container just fine. In particular, |
I can reproduce locally, and for me it seems that https://github.com/PFischbeck/pontoon/blob/django-3.2/pontoon/tags/__init__.py#L1 is the culprit. |
App configs are now automatically discovered, and most of our current app configs were redundant or had wrong names. The DEFAULT_AUTO_FIELD is needed to prevent unwanted migrations in the future, when the default value will change to BigAutoField.
Thanks for the patch! I've deployed to stage and noticed /projects is failing:
|
I bet this is an old |
Turns out this does come from the Django update, and is caused by our own code:
We instantiate the abstract model |
Django 3.2 throws an error when instantiating an abstract model. This commit also adds tests for the views that were previously not covered.
@mathjazz I've pushed a fix, the same error also happened for (I also did some drive-by improvement of test fixture usage in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, just one more thing about test coverage (or the claim of it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified on stage.
Nice work!
This upgrades Pontoon to Django 3.2, which means we should be good to go until April 2024 😄
App configs are now automatically discovered, and most of our current app configs were redundant or had wrong names.
The
DEFAULT_AUTO_FIELD
is needed to prevent unwanted migrations in the future, when the default value will change to BigAutoField.I'll submit a small additional PR to make use of the new
@admin.display
decorator.