[PATCH v2 05/13] REST: Resolve performance issues with tags
Daniel Axtens
dja at axtens.net
Mon Nov 21 14:18:21 AEDT 2016
Hi Stephen,
Always keen on performance improvements! I think the new data format is
better too.
I'm not 100% across the DRF view of the world, so apologies if there are
any dumb questions.
> def get_queryset(self):
> - qs = super(PatchViewSet, self).get_queryset(
> - ).prefetch_related(
> - 'check_set', 'patchtag_set'
> - ).select_related('state', 'submitter', 'delegate')
> + qs = super(PatchViewSet, self).get_queryset().with_tag_counts()\
> + .prefetch_related('check_set')\
> + .select_related('state', 'submitter', 'delegate')
I notice you've added the with_tag_counts. Is that required every time
we get the query_set? What's the motivation for doing it here?
> if 'pk' not in self.kwargs:
> # we are doing a listing, we don't need these fields
> qs = qs.defer('content', 'diff', 'headers')
> diff --git a/patchwork/models.py b/patchwork/models.py
> index a27dda6..d1d0857 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -225,8 +225,8 @@ def get_default_initial_patch_state():
>
> class PatchQuerySet(models.query.QuerySet):
>
> - def with_tag_counts(self, project):
> - if not project.use_tags:
> + def with_tag_counts(self, project=None):
> + if project and not project.use_tags:
> return self
I must admit some confusion here. Previously project was not
nullabe. What's the motivation for making it nullable?
And one final question - given our recent history with broken parsing -
is this all going to behave fine in the presence of malformed tags? I
don't think this particular patch (or patch set) really touch it, but
probably worth thinking about while we're in this area.
Regards,
Daniel
More information about the Patchwork
mailing list