Django 4.0 compatibility #8178

carltongibson · 2021-09-22T08:13:41Z

Adds compatibility for Django 4.0.

From Django 4.0 zoneinfo is the default time zone implementation. This PR makes the minimum changes to work with that, plus other necessary adjustments.

DRF now requires pytz in install_requires. There's a small amount of work to be done in fields.py and test_fields.py to remove the pytz dependency. (Django will remove support for pytz in Django 5.0.)

pytz will not automatically be installed with Django from v4.0.

Refs django/django@faba5b7. addClassCleanup() is available from Python 3.8, which is the minimum supported Python from Django 4.0.

tomchristie

Alrighty! Thanks so much for cracking on with this Carlton. 😀

I've left a few review comments for cases where it wasn't obvious to me why things have changed. More out of the sake of completeness of review, rather than anything that's necessarily a blocker. I'd actually be happy for it to go in either with or without resolution to these. (But who knows perhaps it'll help tease out some 4.0 release notes or something?)

tomchristie · 2021-09-23T09:23:15Z

rest_framework/test.py

+
+        if django.VERSION > (4, 0):
+            cls.addClassCleanup(cls._override.disable)
+            cls.addClassCleanup(cleanup_url_patterns, cls)


Is addClassCleanup a new feature in Django 4.0?

I'm not seeing any differences in the docs re. the existing tearDownClass... https://docs.djangoproject.com/en/4.0/topics/testing/tools/#simpletestcase

addClassCleanup is from unittest.TestCase, added in Python 3.8 https://docs.python.org/3.9/library/unittest.html#unittest.TestCase.addClassCleanup

Django just happens to be using it from Django 4.0 — See django/django@faba5b7

Ah fab. How about we switch based on the Python version, rather than the Django version?

Maybe we should comment on the release notes for this? 🤔 (cc @felixxm )

I don't suppose there are many projects doing anything as subtle as URLPatternsTestCase

Maybe we should comment on the release notes for this? thinking (cc @felixxm )

Sure, if you think it might be helpful. Is it not enough to call super().setUpClass() at the beginning? 🤔

@classmethod def setUpClass(cls): super().setUpClass() # Get the module of the TestCase subclass cls._module = import_module(cls.__module__) ...

It might be... Let me try that after lunch 🥪

... or use cls._overridden_settings and remove redundant cls._override.enable()/disable() calls 🤔

cls._overridden_settings = { 'ROOT_URLCONF': cls.__module__, **cls._overridden_settings, } super().setUpClass()

Moving super().setUpClass() from the bottom of the setUpClass method to the top resolves this without any 4.0 branching.

Do I properly understand why? Nope.

In other words, this works...

@classmethod def setUpClass(cls): super().setUpClass() # <--- This line has moved. # Get the module of the TestCase subclass cls._module = import_module(cls.__module__) cls._override = override_settings(ROOT_URLCONF=cls.__module__) if hasattr(cls._module, 'urlpatterns'): cls._module_urlpatterns = cls._module.urlpatterns cls._module.urlpatterns = cls.urlpatterns cls._override.enable() @classmethod def tearDownClass(cls): super().tearDownClass() cls._override.disable() if hasattr(cls, '_module_urlpatterns'): cls._module.urlpatterns = cls._module_urlpatterns else: del cls._module.urlpatterns

tests/authentication/test_authentication.py

tests/conftest.py

tomchristie · 2021-09-23T09:36:47Z

tests/test_testing.py

+        def tearDownClass(cls):
+            assert urlpatterns is cls.urlpatterns
+            super().tearDownClass()
+            assert urlpatterns is not cls.urlpatterns


Same tearDownClass vs addClassCleanup question as above.

Nothing about it in the release notes... https://docs.djangoproject.com/en/4.0/releases/4.0/ so I'm curious what prompts this to need changing?

See the commit I linked — we no longer call tearDown ourselves so super() isn't doing what it used to. (I presume the base implementation is empty.)

I didn't trace the exact call flow... — timings on those enable/disable or __enter__/__exit__ s is tricky, but there were failures without following the change in Django.

Hrm that's interesting yeah. Looking through the commit in django/django@faba5b7 I can't see any reason why subclasses would need to stop using tearDownClass, even if Django 4.0 isn't using it directly for it's own implementation. But there's possibly some change in interaction I've not figured out there.

Yeah, no 😀 — it's not obvious.

My suspicion it's the timing on the override_settings usage — which is always delicate, as we know from e.g. #5668 &friends.

😜

Once super().setUpClass() is moved in rest_framework/test.py this class doesn't need to change at all.
(Or at least, that's how it seems from my testing.)

My suspicion it's the timing on the override_settings usage — which is always delicate, as we know from e.g. #5668 &friends.

Yes that seems very likely.

Okay, if you want to adjust please do! (Otherwise I'll have another look later on)

carltongibson · 2021-09-23T09:50:33Z

Hey @tomchristie Thanks — I've put initial answers there. Just follow up if you want to dig further. (np my end :)

USE_L10N defaults to True from Django 4.0, and will be removed in Django 5.0.

Private _get_new_csrf_token() was removed in django/django@231de68.

…ions. zoneinfo was made the default time zone implementation in django/django@306607d.

carltongibson · 2021-09-23T12:30:00Z

Grrr.

In other words, this works...

@classmethod
def setUpClass(cls):
     super().setUpClass()  # <--- This line has moved.

So added that in 8eefb21.

It fails on Python 3.7 (and then the rest of the runs fast-fail and cancel)

E   django.core.exceptions.ImproperlyConfigured: The included URLconf 'tests.test_routers' does not appear to have any patterns in it. If you see valid patterns in the file then the issue is probably caused by a circular import.
1357
__________________ test_head_and_options_methods_are_excluded __________________
1358
tests/schemas/test_coreapi.py:1284: in test_head_and_options_methods_are_excluded
1359
    inspector = EndpointEnumerator()
1360
rest_framework/schemas/generators.py:68: in __init__
1361
    patterns = urls.urlpatterns
1362
E   AttributeError: module 'tests.test_routers' has no attribute 'urlpatterns'

Trying different envs locally it looks like the 3.2 to 4.0 change. Just moving the super() call it passes on py38-django40 but fails on py38-django32.

I need to do something else now, but I'll look at @felixxm's suggestions later on.

carltongibson · 2021-09-23T14:21:12Z

OK, likely it's resolvable with enough staring at it, but I can't get it lined up for all versions immediately, so I've reset the PR to its earlier state, which has the branching, but does in fact work.

tomchristie

Okey dokes. I don't think the implementation details on URLPatternsTestCase really matter here, so happy for this to go in whenever. (Important thing is that we probably don't think there's anything that needs adding to the Django release notes, right?)

carltongibson · 2021-09-24T06:14:47Z

Thanks Tom — In that case I think we should get this in, so we're clean against 4.0a1 from our POV. Folks then testing can then turn up any other issues.

carltongibson · 2021-09-24T06:27:21Z

Important thing is that we probably don't think there's anything that needs adding to the Django release notes, right?

I'm not exactly sure what I'd say. :) — But I will chat with @felixxm about it next week.
😉

Added pytz to install_requires.

250479d

pytz will not automatically be installed with Django from v4.0.

carltongibson requested a review from tomchristie September 22, 2021 08:13

carltongibson added 2 commits September 22, 2021 10:23

Adjusted DateTimeField docs for zoneinfo.

f651878

Adjusted URLPatternsTestCase to use addClassCleanup() from Django 4.0.

2d9eee5

Refs django/django@faba5b7. addClassCleanup() is available from Python 3.8, which is the minimum supported Python from Django 4.0.

carltongibson force-pushed the django-4.0-compatibility branch 2 times, most recently from 9822649 to a7e017d Compare September 22, 2021 08:45

tomchristie reviewed Sep 23, 2021

View reviewed changes

carltongibson added 4 commits September 23, 2021 11:57

Removed USE_L10N setting from Django 4.0.

4916854

USE_L10N defaults to True from Django 4.0, and will be removed in Django 5.0.

Adjusted authentication test for internal CSRF changes.

19b6091

Private _get_new_csrf_token() was removed in django/django@231de68.

Fixed TestDefaultTZDateTimeField to allow multiple tzinfo implementat…

f46c33e

…ions. zoneinfo was made the default time zone implementation in django/django@306607d.

Added Django 4.0 to test matrix.

c62e3ca

carltongibson force-pushed the django-4.0-compatibility branch 2 times, most recently from c62e3ca to 3f67343 Compare September 23, 2021 12:15

carltongibson force-pushed the django-4.0-compatibility branch from 3f67343 to c62e3ca Compare September 23, 2021 14:19

tomchristie approved these changes Sep 23, 2021

View reviewed changes

carltongibson merged commit c62e3ca into encode:master Sep 24, 2021

carltongibson deleted the django-4.0-compatibility branch September 24, 2021 06:27

sliverc mentioned this pull request Oct 22, 2021

Add support for Django 4.0 django-json-api/django-rest-framework-json-api#1000

Closed

2 tasks

Zagrebelin mentioned this pull request Dec 14, 2021

Update to Django 4.0 brianjbuck/drf_orjson_renderer#13

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Django 4.0 compatibility #8178

Django 4.0 compatibility #8178

carltongibson commented Sep 22, 2021 •

edited

Loading

tomchristie left a comment

tomchristie Sep 23, 2021

carltongibson Sep 23, 2021

carltongibson Sep 23, 2021

tomchristie Sep 23, 2021

carltongibson Sep 23, 2021 •

edited

Loading

felixxm Sep 23, 2021

carltongibson Sep 23, 2021

felixxm Sep 23, 2021

tomchristie Sep 23, 2021

tomchristie Sep 23, 2021

tomchristie Sep 23, 2021

carltongibson Sep 23, 2021

tomchristie Sep 23, 2021

carltongibson Sep 23, 2021 •

edited

Loading

tomchristie Sep 23, 2021

tomchristie Sep 23, 2021

carltongibson Sep 23, 2021

carltongibson commented Sep 23, 2021

carltongibson commented Sep 23, 2021

carltongibson commented Sep 23, 2021

tomchristie left a comment

carltongibson commented Sep 24, 2021

carltongibson commented Sep 24, 2021

Django 4.0 compatibility #8178

Django 4.0 compatibility #8178

Conversation

carltongibson commented Sep 22, 2021 • edited Loading

tomchristie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carltongibson Sep 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carltongibson Sep 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carltongibson commented Sep 23, 2021

carltongibson commented Sep 23, 2021

carltongibson commented Sep 23, 2021

tomchristie left a comment

Choose a reason for hiding this comment

carltongibson commented Sep 24, 2021

carltongibson commented Sep 24, 2021

carltongibson commented Sep 22, 2021 •

edited

Loading

carltongibson Sep 23, 2021 •

edited

Loading

carltongibson Sep 23, 2021 •

edited

Loading