bug 1467520: bug 1467532: Resolve RemovedInDjango20Warnings #5089
Conversation
MatonAnthony
force-pushed
the
remove-django-warnings
branch
from
8ead5bf to
377ab2d
Compare
|
Please use bug 1467520. |
There was a problem hiding this comment.
Thanks @MatonAnthony, this is a good step forward. It should not wait on django-ratelimit.
I think this should be a merge commit, and the commits should get the bug numbers:
is_authenticated- bug 1467520- django-allauth update - bug 1467532
- Dockerfile change - I wouldn't make this change (I'd set it in my
docker-compose.ymlinstead of the base image), but it can have bug 1467520 since it is commented out.
If you are uncomfortable with a rebase, I can do it.
|
Opened jsocol/django-ratelimit#154 to eliminate the warning. |
- Update django-allauth to remove the warning about is_authenticated() not being a function but a property in Django 2.0. See https://django-allauth.readthedocs.io/en/latest/release-notes.html
- Document which value of PYTHONWARNINGS is needed to display the Python warnings. - Add `no-tty` on GPG command because it's creating issues otherwise.
MatonAnthony
force-pushed
the
remove-django-warnings
branch
from
377ab2d to
81059de
Compare
MatonAnthony
changed the title
|
Sorry it took a while to fix the few nits. I have no hard feelings about the Dockerfile change, but I didn't saw it documented anywhere and I felt like the Dockerfile itself was an appropriate place. So if you want it gone, I can remove the commit. |
There was a problem hiding this comment.
When I take a second look, I see that this line is directly above the new one:
# disable this when preparing for Django upgrade
PYTHONWARNINGS=ignore \
I think it would be OK to have the alternate version in the file as well.
Thanks for the rebase, and thanks for tackling these warnings, @MatonAnthony!
Copy the --no-tty fix from PR mdn#5089
To upgrade Kuma to Django 2.0, we need to address multiple warnings.
PYTHONWARNINGS=allfor documentation purposes.Next steps:
django-ratelimitwhich is responsible for most of the remaining warnings, there is currently no release compatible with Django 2.0 (see: Django 2.0 compatibility jsocol/django-ratelimit#146 however the master tree is compatible with Django 2.0 according to: https://github.com/jsocol/django-ratelimit/pull/136/files)Remaining warnings:
After the
django-ratelimitupdate, the remaining warnings are coming fromdjango-tidingsbut will disappear automatically when we update to Django 2.0 (see: https://github.com/mozilla/django-tidings/blob/master/tidings/compat.py)Personal opinion:
I am not sure it is worth waiting for a
django-ratelimitrelease to merge this, but I believe we should take contact with the module maintainer to see if a release can be made.