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

Automatically determine select_related and prefetch_related on ModelSerializer #52

Closed
cancan101 opened this issue Nov 6, 2014 · 4 comments

Comments

@cancan101
Copy link

What do you think about adding something along the lines of encode/django-rest-framework#1964 to drf-extensions?

@thedrow offers a pretty good write-up on a workable approach: encode/django-rest-framework#1964 (comment)

@chibisov
Copy link
Owner

chibisov commented Nov 6, 2014

Interesting. I think it should be splitted to two stages:

  1. get_prefetch_related_fields and get_select_related_fields serializer's methods.
  2. Mixin for viewset that extends get_queryset method and applies select_related or prefetch_related to queryset based on those serializer's methods
  3. Maybe some automatic magic in serializer itself that turned off by default.

But I would say it's too magic. For example, in most cases I use serializers in ViewSets and use prefetching facilities explicitly:

class UserViewSet(viewsets.ReadOnlyModelViewSet):
    serializer_class = UserSerializer
    queryset = queryset.prefetch_related('groups__permissions')

@kevin-brown
Copy link
Contributor

This may be something that should be in its own third party package, though it would be nice to see it be in drf-extensions.

For example, in most cases I use serializers in ViewSets and use prefetching facilities explicitly:

I wish I could say that most users do this as well, but the Stack Overflow questions keep coming.

@cancan101
Copy link
Author

TBH, I understand how select_related works and I was able to diagnose the N+1 issue as a missing select_related on the queryset, but I was a little surprised at first that drf did not do this automatically when using nested serializer or even just SlugRelatedField. That is why I filed the issue originally on drf.

@chibisov
Copy link
Owner

By and large I think it's too magic. Closing this issue.

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

3 participants