Skip to content

Commit 10bcde5

Browse files
authored
Addonss: use standard Response object and don't sort response (#12334)
The json response isn't meant to be human-readable, sorting and indenting is slow (30 times slower than just returning a normal response). But this only noticeable with big responses, so in reality it won't affect much of the performance. Also, since we are using DRF, we should always return Response objects, they are the recommended way of writing APIs. I wasn't able to remove one usage of JsonResponse, but we should use an exception instead like suggested in https://stackoverflow.com/questions/46713047/django-rest-framework-return-response-from-dispatch, I didn't want to make a backwards compatible change here.
1 parent 9f091b6 commit 10bcde5

File tree

1 file changed

+7
-4
lines changed

1 file changed

+7
-4
lines changed

readthedocs/proxito/views/hosting.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from django.shortcuts import get_object_or_404
1212
from rest_framework import permissions
1313
from rest_framework.renderers import JSONRenderer
14+
from rest_framework.response import Response
1415
from rest_framework.views import APIView
1516

1617
from readthedocs.api.mixins import CDNCacheTagsMixin
@@ -172,6 +173,8 @@ def dispatch(self, request, *args, **kwargs):
172173
project_slug = request.GET.get("project-slug")
173174
version_slug = request.GET.get("version-slug")
174175
if not url and (not project_slug or not version_slug):
176+
# NOTE: we don't use Response because we can't return it from
177+
# the dispatch method, we shuould refactor this to raise a subclass of APIException instead.
175178
return JsonResponse(
176179
{
177180
"error": "'project-slug' and 'version-slug' GET attributes are required when not sending 'url'"
@@ -184,7 +187,7 @@ def get(self, request, *args, **kwargs):
184187
url = request.GET.get("url")
185188
addons_version = request.GET.get("api-version")
186189
if not addons_version:
187-
return JsonResponse(
190+
return Response(
188191
{"error": "'api-version' GET attribute is required"},
189192
status=400,
190193
)
@@ -193,14 +196,14 @@ def get(self, request, *args, **kwargs):
193196
if addons_version.major not in ADDONS_VERSIONS_SUPPORTED:
194197
raise ClientError
195198
except packaging.version.InvalidVersion:
196-
return JsonResponse(
199+
return Response(
197200
{
198201
"error": ClientError.VERSION_INVALID,
199202
},
200203
status=400,
201204
)
202205
except ClientError:
203-
return JsonResponse(
206+
return Response(
204207
{"error": ClientError.VERSION_NOT_CURRENTLY_SUPPORTED},
205208
status=400,
206209
)
@@ -216,7 +219,7 @@ def get(self, request, *args, **kwargs):
216219
filename=filename,
217220
url=url,
218221
)
219-
return JsonResponse(data, json_dumps_params={"indent": 4, "sort_keys": True})
222+
return Response(data)
220223

221224

222225
class NoLinksMixin:

0 commit comments

Comments
 (0)