Monkeypatching breaks DRF filters

[jerivas]

Bug report

What's wrong

It appears that mokeypatching DRF viewsets (as suggested here) breaks the usage of django_filters FilterSets:

# views.py
class DocumentViewSet(viewsets.ModelViewSet[Document]):
    queryset = Document.objects.all()
    serializer_class = DocumentSerializer
    filterset_class = DocumentFilterSet

# settings.py
import django_stubs_ext
from rest_framework import viewsets

django_stubs_ext.monkeypatch(extra_classes=(viewsets.ModelViewSet,))

With this code the DocumentViewSet completely ignores the filters defined in DocumentFilterSet. GET params simply stop working for filtering and the filter UI is completely missing from DRF's HTML view.

Refer to this test in the demo repository. In the original commit, the test passes, but after types are added, the test fails.

How is that should be

DRF filters should continue working after monkeypatching.

System information

• OS: Ubuntu 20.04
• python version: 3.10
• django version: 4.1
• mypy version: 0.991
• django-stubs version: 1.13.0
• django-stubs-ext version: 0.7.0

[LucidDan]

This happened to me this week on a production app for a client. We realised the problem because our Zapier integration stopped working when the REST API no longer had search and pagination support. Took me a whole day to track the problem down.

Reproduction

If anyone wants to see minimal repro - https://github.com/LucidDan/drf-repro

1. Run it using DJANGO_SETTINGS_MODULE=tutorial.settings ; everything is fine
2. Run it using DJANGO_SETTINGS_MODULE=tutorial.broken_settings ; everything is fine, apparently! ...but no pagination enabled.

The only difference between the two settings files is broken_settings has: from rest_framework import generics. No djangorestframework-stubs installed in the reproduction, because the problem can be triggered without it. It's not a bug in drf-stubs.

Root Cause

I think what is actually happening here is that importing from rest_framework in the settings.py causes django-rest-framework to fail to load any settings and just use its defaults. Unfortunately, it does this silently, so you can easily end up with things breaking without realising it.

I think this is not really a djangorestframework-stubs bug. It's maybe not even really a rest-framework "bug" - more of a feature of past design choices (eg choosing to allow REST_FRAMEWORK to be optional and not throw an error if it isn't defined).

Workarounds

The workarounds are not ideal, and involve some level of compromise:

1. There is "from rest_framework.settings import api_settings; api_settings.reload()". In theory, this lets you reload the settings, but in practice I did not have any success with this approach. I suspect by the time you do the reload(), a bunch of rest_framework code has already loaded and is using the defaults. Which raises other questions about why it even exists, but that's another issue.
2. You can monkeypatch later in settings.py, after REST_FRAMEWORK has been defined. This seems to work. Beware if you have any custom _CLASSES in the REST_FRAMEWORK variable, if the python modules they are defined in have type hints you might hit subscripts before the monkeypatching has been applied.
3. You can do your django-stubs monkeypatch in settings.py (without the extra_classes), and monkeypatch the DRF classes in the python modules where you are using rest-framework code. The downside being you'll have to repeat this multiple times.

You might also be able to get away with monkeypatching in an apps.py file or something, so that it gets monkeypatched AFTER settings has been loaded fully...but I found this approach a bit too brittle. There's too much dependent on the order stuff gets imported.

For now, I'm using option 2 - it's ugly to late-import stuff at the end of settings.py, but at least its DRY-er than monkeypatching in every file I use rest-framework.

Permanent Fix

For fixing this properly, I have no idea how it can be done on the drf-stubs side; it seems more like a rest-framework issue, and for -stubs, it should just be documented more clearly that you need to apply monkeypatching and that there are some potential gotchas in doing so.

[jalaziz]

I've run into this issue too. In my case, authentication classes aren't getting configured and request.user is always returning AnonymousUser.

Unfortunately, importing rest_framework later in settings (after REST_FRAMEWORK is defined) doesn't seem to help.

[jalaziz]

I've submitted encode/django-rest-framework#8825 in hopes that it would solve this problem at the root.

[browniebroke]

I think I've got the same problem with https://github.com/vbabiy/djangorestframework-camel-case, when I apply the monkeypatching, my DEFAULT_RENDERER_CLASSES setting is ignored, and it falls back to the default snake case renderer.

Thanks for the thourough explanation above, I think it all makes sense. Unfortunately, the second option in the workarounds don't work in my case. Given that the fix in DRF is merged is hopefully going to be released soon, I'll probably wait for it. Thanks for the great work @jalaziz!

[justuswilhelm]

I can't believe there are others who have the same issue! Yes, DRF loading the settings "prematurely" causes the issue. I reverted to add class_getitem to Django's generic.View instead. Django is not too picky about importing Views early. It works because of the inheritance View > APIView > GenericAPIView. Is it clean and correct? No. But it works, and I will wait until the next DRF release.

Here's my code:

from django.views import generic  # For patching

patchable_classes  = (
    generic.View,
)


def patch() -> None:
    """Patch classes."""
    for cls in patchable_classes:
        cls.__class_getitem__ = classmethod(lambda cls, *args, **kwargs: cls)

[mschoettle]

I ran into the same issue after monkeypatching Field and GenericAPIView that requests were always unauthenticated. The Authorization header is not present in request.META.