diff mbox series

views: Use select_related, not prefetch_related

Message ID 20181029154639.20589-1-stephen@that.guru
State Accepted
Headers show
Series views: Use select_related, not prefetch_related | expand

Commit Message

Stephen Finucane Oct. 29, 2018, 3:46 p.m. UTC
Now that there's a 1:N rather than N:M relationship between series and
patches, we should be using select_related rather than prefetch_related.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Closes: #223
Fixes: 76505e91 ("models: Convert Series-Patch relationship to 1:N")
---
 patchwork/views/__init__.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Daniel Axtens Oct. 30, 2018, 1:20 p.m. UTC | #1
Stephen Finucane <stephen@that.guru> writes:

> Now that there's a 1:N rather than N:M relationship between series and
> patches, we should be using select_related rather than prefetch_related.
>
Tested-by: Daniel Axtens <dja@axtens.net>

And applied, thank you!
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Closes: #223
> Fixes: 76505e91 ("models: Convert Series-Patch relationship to 1:N")
> ---
>  patchwork/views/__init__.py | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
> index 178b3d41..cea61486 100644
> --- a/patchwork/views/__init__.py
> +++ b/patchwork/views/__init__.py
> @@ -273,17 +273,16 @@ def generic_list(request, project, view, view_args=None, filter_settings=None,
>  
>      # but we will need to follow the state and submitter relations for
>      # rendering the list template
> -    patches = patches.select_related('state', 'submitter', 'delegate')
> +    patches = patches.select_related('state', 'submitter', 'delegate',
> +                                     'series')
>  
>      patches = patches.only('state', 'submitter', 'delegate', 'project',
> -                           'name', 'date')
> +                           'series__name', 'name', 'date')
>  
>      # we also need checks and series
>      patches = patches.prefetch_related(
>          Prefetch('check_set', queryset=Check.objects.only(
>              'context', 'user_id', 'patch_id', 'state', 'date')))
> -    patches = patches.prefetch_related(
> -        Prefetch('series', queryset=Series.objects.only('name')))
>  
>      paginator = Paginator(request, patches)
>  
> -- 
> 2.17.2
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
Daniel Axtens Oct. 30, 2018, 1:47 p.m. UTC | #2
Daniel Axtens <dja@axtens.net> writes:

> Stephen Finucane <stephen@that.guru> writes:
>
>> Now that there's a 1:N rather than N:M relationship between series and
>> patches, we should be using select_related rather than prefetch_related.
>>
> Tested-by: Daniel Axtens <dja@axtens.net>
>
> And applied, thank you!
Much to my shame this breaks pep8 and I forgot to check before applying.

I have applied the following fixup:

commit ee20fdea757e393f5951aaf640331176fc6cb807 (HEAD, upstream-push/master)
Author: Daniel Axtens <dja@axtens.net>
Date:   Wed Oct 31 00:45:44 2018 +1100

    views: no longer import Series in __init__.py
    
    Fix a pep8 error due to an unused import.
    
    Fixes: 780b1a2d07ad ("views: Use select_related, not prefetch_related")
    Signed-off-by: Daniel Axtens <dja@axtens.net>

diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
index cea614862d1e..6ec53ddacbe9 100644
--- a/patchwork/views/__init__.py
+++ b/patchwork/views/__init__.py
@@ -14,7 +14,6 @@ from patchwork.models import BundlePatch
 from patchwork.models import Patch
 from patchwork.models import Project
 from patchwork.models import Check
-from patchwork.models import Series
 from patchwork.paginator import Paginator
 
 


>> Signed-off-by: Stephen Finucane <stephen@that.guru>
>> Closes: #223
>> Fixes: 76505e91 ("models: Convert Series-Patch relationship to 1:N")
>> ---
>>  patchwork/views/__init__.py | 7 +++----
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
>> index 178b3d41..cea61486 100644
>> --- a/patchwork/views/__init__.py
>> +++ b/patchwork/views/__init__.py
>> @@ -273,17 +273,16 @@ def generic_list(request, project, view, view_args=None, filter_settings=None,
>>  
>>      # but we will need to follow the state and submitter relations for
>>      # rendering the list template
>> -    patches = patches.select_related('state', 'submitter', 'delegate')
>> +    patches = patches.select_related('state', 'submitter', 'delegate',
>> +                                     'series')
>>  
>>      patches = patches.only('state', 'submitter', 'delegate', 'project',
>> -                           'name', 'date')
>> +                           'series__name', 'name', 'date')
>>  
>>      # we also need checks and series
>>      patches = patches.prefetch_related(
>>          Prefetch('check_set', queryset=Check.objects.only(
>>              'context', 'user_id', 'patch_id', 'state', 'date')))
>> -    patches = patches.prefetch_related(
>> -        Prefetch('series', queryset=Series.objects.only('name')))
>>  
>>      paginator = Paginator(request, patches)
>>  
>> -- 
>> 2.17.2
>>
>> _______________________________________________
>> Patchwork mailing list
>> Patchwork@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/patchwork
Stephen Finucane Oct. 30, 2018, 1:54 p.m. UTC | #3
On Wed, 2018-10-31 at 00:47 +1100, Daniel Axtens wrote:
> Daniel Axtens <dja@axtens.net> writes:
> 
> > Stephen Finucane <stephen@that.guru> writes:
> > 
> > > Now that there's a 1:N rather than N:M relationship between series and
> > > patches, we should be using select_related rather than prefetch_related.
> > > 
> > 
> > Tested-by: Daniel Axtens <dja@axtens.net>
> > 
> > And applied, thank you!
> 
> Much to my shame this breaks pep8 and I forgot to check before applying.
> 
> I have applied the following fixup:

Good catch. I need to add pre-push hook for this stuff. Note that the
build is also broken because of the PostgreSQL 11 issue. There's a
patch on the list to fix this.

Stephen
diff mbox series

Patch

diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
index 178b3d41..cea61486 100644
--- a/patchwork/views/__init__.py
+++ b/patchwork/views/__init__.py
@@ -273,17 +273,16 @@  def generic_list(request, project, view, view_args=None, filter_settings=None,
 
     # but we will need to follow the state and submitter relations for
     # rendering the list template
-    patches = patches.select_related('state', 'submitter', 'delegate')
+    patches = patches.select_related('state', 'submitter', 'delegate',
+                                     'series')
 
     patches = patches.only('state', 'submitter', 'delegate', 'project',
-                           'name', 'date')
+                           'series__name', 'name', 'date')
 
     # we also need checks and series
     patches = patches.prefetch_related(
         Prefetch('check_set', queryset=Check.objects.only(
             'context', 'user_id', 'patch_id', 'state', 'date')))
-    patches = patches.prefetch_related(
-        Prefetch('series', queryset=Series.objects.only('name')))
 
     paginator = Paginator(request, patches)