Styria-Digital / django-rest-framework-jwt

JSON Web Token Authentication support for Django REST Framework
https://styria-digital.github.io/django-rest-framework-jwt/
MIT License
192 stars 60 forks source link

Deprecation Warnings In Django 3 #43

Closed dominik-bln closed 4 years ago

dominik-bln commented 4 years ago

We recently updated from Django 2 to Django 3 and are now seeing quite a few deprecation warnings regarding RemovedInDjango40Warning: django.utils.translation.ugettext() is deprecated in favor of django.utils.translation.gettext() coming from this package.

I believe this is caused by the try-catch block in compat.py being "reused" for multiple imports where the first line seems to fail in Django 3:

https://github.com/Styria-Digital/django-rest-framework-jwt/blob/master/src/rest_framework_jwt/compat.py#L12

It looks like url is not part of django.conf and therefore always raises ImportError:

https://docs.djangoproject.com/en/3.0/ref/urls/

I believe the following should fix the problem in compat.py, I'm just not completely sure if the middle block even makes sense or of it should be completely removed:

try:
    from django.urls import include
except ImportError:
    from django.conf.urls import include  # noqa: F401

try:
  from django.urls import url
except ImportError:
  from django.conf.urls import url

try:
    from django.utils.translation import gettext as gettext_lazy
except ImportError:
    from django.utils.translation import ugettext as gettext_lazy
fitodic commented 4 years ago

@dominik-bln Thanks for reporting this. :slightly_smiling_face:

You can merge the first two try/except blocks. Could you please send a PR with this change?

dominik-bln commented 4 years ago

Ok, will do.

dominik-bln commented 4 years ago

I've opened https://github.com/Styria-Digital/django-rest-framework-jwt/pull/45 to address this.

@fitodic I've checked again regarding merging the two blocks. From the Django v3 docs it looks like include should come from django.urls and url from django.conf.urls. I've switched that around now, otherwise a similar problem could likely crop up again when importing url from django.urls gets deprecated.

dominik-bln commented 4 years ago

Added one more to fix a deprecation of force_text in https://github.com/Styria-Digital/django-rest-framework-jwt/pull/46

fitodic commented 4 years ago

I've opened #45 to address this.

@fitodic I've checked again regarding merging the two blocks. From the Django v3 docs it looks like include should come from django.urls and url from django.conf.urls. I've switched that around now, otherwise a similar problem could likely crop up again when importing url from django.urls gets deprecated.

Great :+1: