django-waffle icon indicating copy to clipboard operation
django-waffle copied to clipboard

Waffle can't be used directly on DRF's request objects

Open machine-mind opened this issue 8 years ago • 5 comments

We are building a Django Web Application using Django Rest Framework and AngularJS. We would like to add feature flipping (feature flags) to our framework. Our framework uses token authentication.

We have a problem when we use feature flags based on percentage of users. Waffle appends the attribute "waffles" to the request object. But the attribute does not exist when processed by the waffle middleware.

The request object we are sending is a rest_framework.request.Request type. In the middleware we get a request of type django.core.handlers.wsgi.WSGIRequest

See: http://stackoverflow.com/questions/38373474/is-django-waffle-suited-to-work-with-django-rest-framework-drf

machine-mind avatar Jul 15 '16 13:07 machine-mind

I want to make sure I'm following this correctly, the issue is something like this:

class MyView(rest_framework.APIView):
    def get(self, request, id=None):
        if waffle.flag_is_active(request, 'foo'):  # request is a rest_framework.request.Request
            pass
        return Response(stuff, etc=other_stuff)

Waffle will create the .waffles attribute on request, but then the request object you get within the view isn't the same as the request object waffle sees at the middleware layer? Am I getting that right?'

The problem is that this gets into weird DRF internals that aren't public APIs and are subject to change. I think the original, django.http.HttpRequest is available as request._request within a DRF view, and I'm guessing (or maybe hoping) that when the whole DRF stack is done, that's the same request object it passes back to the Django middleware stack.

What happens if you do flag_is_active(request._request, 'foo')?

If that works, I'm not sure what to do. I really don't like the idea of mucking with internals or accepting a bunch of code in waffle to deal with the problems DRF creates with its Request class. If anything, maybe it could live in a waffle.contrib.drf module. Or maybe just document that as a not-very-nice solution.

jsocol avatar Jul 16 '16 14:07 jsocol

We tried flag_is_active(request._request, 'foo') and it works fine. Should I close this bug request, then?

machine-mind avatar Jul 28 '16 10:07 machine-mind

Nah, leave it open for now. Maybe it turns into documentation or a contrib.drf module at some point.

jsocol avatar Jul 28 '16 11:07 jsocol

👋 hi folks from #295.

The problem I identified in this comment https://github.com/jsocol/django-waffle/issues/221#issuecomment-233133143 may not be the most important part of the issue. If https://github.com/jsocol/django-waffle/issues/1#issuecomment-371245723 is implemented, I don't believe there will be a need to store the .waffles attribute anymore—and potentially the .waffle_tests code could get rethought or eliminated (for automated testing it might be ok to tell a browser "set the cookie yourself" e.g. via selenium).

I still would rather keep DRF-specific code segregated—especially if there are optional requirements. I'd rather not duck-type DRF Request objects, for example—in a contrib.drf/ext.drf or similar place where it's explicit, and where we could import from DRF without making it a dependency.

jsocol avatar May 03 '18 21:05 jsocol

Coming over from #295. Myself and @martinburchell are using the if waffle.flag_is_active(request, 'foo') dance in the function body of our ViewSet and we'd like to be able to use the decorator. That's about it. Personally, I think DRF should be a first class citizen for all 3rd party packages that work on the API level because it is so widely used.

I still would rather keep DRF-specific code segregated—especially if there are optional requirements. I'd rather not duck-type DRF Request objects, for example—in a contrib.drf/ext.drf or similar place where it's explicit, and where we could import from DRF without making it a dependency.

That all sounds sensible!

We could probably work on a PR for this if we get a green light. Is there some blocking issues on this?

decentral1se avatar May 04 '18 07:05 decentral1se