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

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

Open
machine-mind opened this issue Jul 15, 2016 · 5 comments
Open

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

machine-mind opened this issue Jul 15, 2016 · 5 comments

Comments

@machine-mind
Copy link

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

@jsocol
Copy link
Collaborator

jsocol commented Jul 16, 2016

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.

@machine-mind
Copy link
Author

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

@jsocol
Copy link
Collaborator

jsocol commented Jul 28, 2016

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

@jsocol jsocol changed the title "waffles" attribute gets dropped, when using percentage flags Waffle can't be used directoy on DRF's request objects Feb 15, 2018
@jsocol jsocol changed the title Waffle can't be used directoy on DRF's request objects Waffle can't be used directly on DRF's request objects Feb 15, 2018
@jsocol
Copy link
Collaborator

jsocol commented May 3, 2018

👋 hi folks from #295.

The problem I identified in this comment #221 (comment) may not be the most important part of the issue. If #1 (comment) 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.

@decentral1se
Copy link

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?

kellyi pushed a commit to opensupplyhub/open-apparel-registry that referenced this issue Oct 17, 2019
Update facility history endpoint tests to use the
`can_get_facility_history` feature flag.

Update facility history endpoint code to replace switch with flag.

Replace endpoint decorator with custom flag_is_active check to
circumvent a quirk with using waffle & DRF documented here:

jazzband/django-waffle#221
rajadain pushed a commit to opensupplyhub/open-apparel-registry that referenced this issue Oct 24, 2019
Update facility history endpoint tests to use the
`can_get_facility_history` feature flag.

Update facility history endpoint code to replace switch with flag.

Replace endpoint decorator with custom flag_is_active check to
circumvent a quirk with using waffle & DRF documented here:

jazzband/django-waffle#221
rajadain pushed a commit to opensupplyhub/open-apparel-registry that referenced this issue Oct 24, 2019
Update facility history endpoint tests to use the
`can_get_facility_history` feature flag.

Update facility history endpoint code to replace switch with flag.

Replace endpoint decorator with custom flag_is_active check to
circumvent a quirk with using waffle & DRF documented here:

jazzband/django-waffle#221
jwalgran pushed a commit to opensupplyhub/open-apparel-registry that referenced this issue Oct 24, 2019
Update facility history endpoint tests to use the
`can_get_facility_history` feature flag.

Update facility history endpoint code to replace switch with flag.

Replace endpoint decorator with custom flag_is_active check to
circumvent a quirk with using waffle & DRF documented here:

jazzband/django-waffle#221
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants