-
Notifications
You must be signed in to change notification settings - Fork 398
[django] upgraded django to 4.1 #3945
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
Conversation
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.
looks good.
b6ef65f
to
e1a6f9b
Compare
ac0a9dd
to
1ab7b15
Compare
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.
Added few review comments
a87ba3c
to
cb0f365
Compare
Backend Code Coverage Report •
Pytest Report
|
cb0f365
to
a77200e
Compare
|
Backend Code Coverage Report •
Pytest Report
|
a77200e
to
6ab0d26
Compare
|
Several other libraries have also been upgraded along with it. Four code changes had to be made. 1. Django 4 wants the middlewares to be explicitly sync and async. Rather than rewriting all of the middleware classes, I took the shortcut of creating a new mixin class that supports only sync calls. 2. CSRF_TRUSTED_ORIGINS in django 4+ need the scheme part as well. We now check and add `http` and `https` to the origins. 3. django-babel also needs a change every time we do a django upgrade. This is mostly just changes around django version support.
6ab0d26
to
fdb4095
Compare
|
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.
LGTM!
I think this broke downloads: #4086 |
Several other libraries have also been upgraded along with it. Three code changes had to be made.
Django 4 wants the middlewares to be explicitly sync and async. Rather than rewriting all of the middleware classes, I took the shortcut of creating a new mixin class that supports only sync calls.
CSRF_TRUSTED_ORIGINS in django 4+ need the scheme part as well. We now check and add
http
andhttps
to the origins.django-babel also needs a change every time we do a django upgrade. This is mostly just changes around django version support.
There is also some change to our database support due to this. Following is the list of supported db versions.
What changes were proposed in this pull request?
How was this patch tested?
Please review Hue Contributing Guide before opening a pull request.