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

Deprecation Warnings In Django 3 #43

Closed
dominik-bln opened this issue Apr 17, 2020 · 5 comments
Closed

Deprecation Warnings In Django 3 #43

dominik-bln opened this issue Apr 17, 2020 · 5 comments

Comments

@dominik-bln
Copy link

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
Copy link
Collaborator

fitodic commented Apr 20, 2020

@dominik-bln Thanks for reporting this. 🙂

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

@dominik-bln
Copy link
Author

Ok, will do.

@dominik-bln
Copy link
Author

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.

@dominik-bln
Copy link
Author

Added one more to fix a deprecation of force_text in #46

@fitodic
Copy link
Collaborator

fitodic commented Apr 23, 2020

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 👍

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

No branches or pull requests

2 participants