diff mbox series

[1/2] api: do not fetch every patch in a patch detail view 404

Message ID 20200414062102.6798-2-dja@axtens.net
State Accepted
Headers show
Series v2.2 fixups for OzLabs | expand

Commit Message

Daniel Axtens April 14, 2020, 6:21 a.m. UTC
mpe and jk and sfr found that the OzLabs server was melting due
to some queries downloading every patch.

Turns out if you 404 the patch detail view in the API, d-r-f attempts
to render a listbox with every single patch to fill in the 'related'
field. The bundle API also has a similar field.

Replace the multiple selection box with a text field. You can still
(AIUI) populate the relevant patch IDs manually.

Reported-by: Jeremy Kerr <jk@ozlabs.org>
Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 patchwork/api/bundle.py   | 3 ++-
 patchwork/api/embedded.py | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

Jeremy Kerr April 14, 2020, 7:27 a.m. UTC | #1
Hi Daniel,

> mpe and jk and sfr found that the OzLabs server was melting due
> to some queries downloading every patch.
> 
> Turns out if you 404 the patch detail view in the API, d-r-f attempts
> to render a listbox with every single patch to fill in the 'related'
> field.

... and the query for that includes all patch content and headers, so
transferring gigabytes of data per access.

>  The bundle API also has a similar field.
> 
> Replace the multiple selection box with a text field. You can still
> (AIUI) populate the relevant patch IDs manually.

Much better, the patchwork server is no longer on fire!

Thanks for that.

Tested-by: Jeremy Kerr <jk@ozlabs.org>
Server-no-longer-on-fire-by: Jeremy Kerr <jk@ozlabs.org>

Cheers,


Jeremy
Stephen Finucane April 14, 2020, 10:22 a.m. UTC | #2
On Tue, 2020-04-14 at 16:21 +1000, Daniel Axtens wrote:
> mpe and jk and sfr found that the OzLabs server was melting due
> to some queries downloading every patch.
> 
> Turns out if you 404 the patch detail view in the API, d-r-f attempts
> to render a listbox with every single patch to fill in the 'related'
> field. The bundle API also has a similar field.
> 
> Replace the multiple selection box with a text field. You can still
> (AIUI) populate the relevant patch IDs manually.
> 
> Reported-by: Jeremy Kerr <jk@ozlabs.org>
> Reported-by: Michael Ellerman <mpe@ellerman.id.au>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Daniel Axtens <dja@axtens.net>

It's a crying shame that DRF doesn't provide any AJAX'y widgets out of
the box, though I guess that would involve choosing a framework since
JQuery is dying. Given they don't, this is the correct solution. Even
the docs [1] say so!

Reviewed-by: Stephen Finucane <stephen@that.guru>

Stephen

PS: It would be nice to include [1] in the commit message when merging.

[1] https://www.django-rest-framework.org/topics/browsable-api/#handling-choicefield-with-large-numbers-of-items
diff mbox series

Patch

diff --git a/patchwork/api/bundle.py b/patchwork/api/bundle.py
index b8c0f1781786..54a9266e7d73 100644
--- a/patchwork/api/bundle.py
+++ b/patchwork/api/bundle.py
@@ -62,7 +62,8 @@  class BundleSerializer(BaseHyperlinkedModelSerializer):
     project = ProjectSerializer(read_only=True)
     mbox = SerializerMethodField()
     owner = UserSerializer(read_only=True)
-    patches = PatchSerializer(many=True, required=True)
+    patches = PatchSerializer(many=True, required=True,
+                              style={'base_template': 'input.html'})
 
     def get_web_url(self, instance):
         request = self.context.get('request')
diff --git a/patchwork/api/embedded.py b/patchwork/api/embedded.py
index 85a30cae1cdf..cb3f07e6b998 100644
--- a/patchwork/api/embedded.py
+++ b/patchwork/api/embedded.py
@@ -141,7 +141,8 @@  class PatchSerializer(SerializedRelatedField):
 
 class PatchRelationSerializer(BaseHyperlinkedModelSerializer):
     """Hide the PatchRelation model, just show the list"""
-    patches = PatchSerializer(many=True)
+    patches = PatchSerializer(many=True,
+                              style={'base_template': 'input.html'})
 
     def to_internal_value(self, data):
         if not isinstance(data, type([])):