From 104d10a15f7c0dfbe5d13c07aee9591f265a0ef2 Mon Sep 17 00:00:00 2001
From: Erwan Rouchet <rouchet@teklia.com>
Date: Thu, 15 Oct 2020 11:50:47 +0200
Subject: [PATCH] Optimize Revision.state and ListWorkerVersions
---
arkindex/dataimport/api.py | 8 +++++---
arkindex/dataimport/models.py | 23 ++++++++++++++++------
arkindex/dataimport/serializers/workers.py | 5 +++++
arkindex/dataimport/tests/test_workers.py | 9 ++++++---
4 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/arkindex/dataimport/api.py b/arkindex/dataimport/api.py
index dfb651edbd..9db544c1ba 100644
--- a/arkindex/dataimport/api.py
+++ b/arkindex/dataimport/api.py
@@ -694,9 +694,11 @@ class WorkerVersionList(ListCreateAPIView):
}
def get_queryset(self):
- return WorkerVersion.objects.filter(
- worker_id=self.kwargs['pk']
- ).prefetch_related('revision').order_by('-revision__created')
+ return WorkerVersion.objects \
+ .filter(worker_id=self.kwargs['pk']) \
+ .select_related('revision__repo', 'worker__repository') \
+ .prefetch_related('revision__refs', 'revision__versions') \
+ .order_by('-revision__created')
def create(self, request, *args, **kwargs):
serializer = self.get_serializer(data=request.data)
diff --git a/arkindex/dataimport/models.py b/arkindex/dataimport/models.py
index f7e7c91106..19f703e1c3 100644
--- a/arkindex/dataimport/models.py
+++ b/arkindex/dataimport/models.py
@@ -352,13 +352,24 @@ class Revision(IndexableModel):
@property
def state(self):
- # Computes revision state according to its versions one
+ """
+ Computes revision state according to its versions'
- # If there is one version in error, revision state is too
- # Else if there is one version processing, revision state is too
- # Else if all versions are available, then the revision is too
- # Else, the revision is created since it has either no version or versions mixing processing/created states
- states = set(self.versions.values_list('state', flat=True))
+ If there is one version in error, revision state is too
+ Else if there is one version processing, revision state is too
+ Else if all versions are available, then the revision is too
+ Else, the revision is created since it has either no version or versions mixing processing/created states
+ """
+ # This prevents performing another SQL request when versions have already been prefetched.
+ # See https://stackoverflow.com/a/19651840/5990435
+ if (
+ hasattr(self, "_prefetched_objects_cache")
+ and self.versions.field.remote_field.get_cache_name()
+ in self._prefetched_objects_cache
+ ):
+ states = set(version.state for version in self.versions.all())
+ else:
+ states = set(self.versions.values_list('state', flat=True))
if WorkerVersionState.Error in states:
return WorkerVersionState.Error
diff --git a/arkindex/dataimport/serializers/workers.py b/arkindex/dataimport/serializers/workers.py
index 9598539dca..71d4405fb0 100644
--- a/arkindex/dataimport/serializers/workers.py
+++ b/arkindex/dataimport/serializers/workers.py
@@ -46,6 +46,11 @@ class WorkerVersionSerializer(serializers.ModelSerializer):
'worker',
)
read_only_fields = ('docker_image_name',)
+ # Avoid loading all revisions and all Ponos artifacts when opening this endpoint in a browser
+ extra_kwargs = {
+ 'revision': {'style': {'base_template': 'input.html'}},
+ 'docker_image': {'style': {'base_template': 'input.html'}},
+ }
def to_representation(self, instance):
self.fields['revision'] = RevisionWithRefsSerializer(read_only=True)
diff --git a/arkindex/dataimport/tests/test_workers.py b/arkindex/dataimport/tests/test_workers.py
index f0101cc058..0fe294824d 100644
--- a/arkindex/dataimport/tests/test_workers.py
+++ b/arkindex/dataimport/tests/test_workers.py
@@ -162,12 +162,14 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
# Tests on get_query_set for WorkerVersionList
def test_versions_list_requires_login(self):
- response = self.client.get(reverse('api:worker-versions', kwargs={'pk': str(self.worker_1.id)}))
+ with self.assertNumQueries(0):
+ response = self.client.get(reverse('api:worker-versions', kwargs={'pk': str(self.worker_1.id)}))
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
def test_versions_list(self):
self.client.force_login(self.user)
- response = self.client.get(reverse('api:worker-versions', kwargs={'pk': str(self.worker_1.id)}))
+ with self.assertNumQueries(5):
+ response = self.client.get(reverse('api:worker-versions', kwargs={'pk': str(self.worker_1.id)}))
self.assertEqual(response.status_code, status.HTTP_200_OK)
data = response.json()
self.assertEqual(len(data), 1)
@@ -190,7 +192,8 @@ class TestWorkersWorkerVersions(FixtureAPITestCase):
configuration={"test": "test2"}
)
- response = self.client.get(reverse('api:worker-versions', kwargs={'pk': str(worker_2.id)}))
+ with self.assertNumQueries(5):
+ response = self.client.get(reverse('api:worker-versions', kwargs={'pk': str(worker_2.id)}))
self.assertEqual(response.status_code, status.HTTP_200_OK)
data = response.json()
self.assertEqual(len(data), 1)
--
GitLab