From dc1527c2840c3b81e2b3dcfd99569226af8be983 Mon Sep 17 00:00:00 2001
From: Erwan Rouchet <rouchet@teklia.com>
Date: Tue, 19 Nov 2024 12:54:55 +0000
Subject: [PATCH] Remove the WorkerVersionManager.get_by_feature cache
---
arkindex/process/admin.py | 14 --------------
arkindex/process/managers.py | 3 +--
arkindex/process/models.py | 5 +++--
arkindex/process/tasks.py | 2 +-
arkindex/process/tests/process/test_start.py | 2 +-
arkindex/process/tests/test_templates.py | 10 +++++-----
arkindex/project/tests/__init__.py | 5 -----
7 files changed, 11 insertions(+), 30 deletions(-)
diff --git a/arkindex/process/admin.py b/arkindex/process/admin.py
index cac9fdfef2..d43d5ae745 100644
--- a/arkindex/process/admin.py
+++ b/arkindex/process/admin.py
@@ -119,20 +119,6 @@ class WorkerVersionAdmin(admin.ModelAdmin):
)
readonly_fields = ("id", )
- def save_model(self, request, obj, form, change):
- # When a WorkerVersion is created with a feature, or an existing one has its feature updated, clear the cached versions providing features
- if form["feature"]._has_changed():
- # `cache_clear` is a function defined by the `functools.lru_cache` decorator
- # on the function itself, not on its return value
- WorkerVersion.objects.get_by_feature.cache_clear()
- super().save_model(request, obj, form, change)
-
- def delete_model(self, request, obj):
- # When this WorkerVersion provides an Arkindex feature, clear the cached versions providing features
- if obj.feature is not None:
- WorkerVersion.objects.get_by_feature.cache_clear()
- super().delete_model(request, obj)
-
class WorkerConfigurationAdmin(admin.ModelAdmin):
list_display = ("id", "name", "worker")
diff --git a/arkindex/process/managers.py b/arkindex/process/managers.py
index 7263aed19f..8bd8ae9e69 100644
--- a/arkindex/process/managers.py
+++ b/arkindex/process/managers.py
@@ -1,6 +1,6 @@
import logging
import operator
-from functools import lru_cache, reduce
+from functools import reduce
from django.db import connections
from django.db.models import Exists, Manager, ManyToOneRel, OuterRef, Q
@@ -162,7 +162,6 @@ class WorkerResultSourceQuerySet(QuerySet):
class WorkerVersionManager(Manager):
- @lru_cache
def get_by_feature(self, feature):
return self.get_queryset().get_by_feature(feature)
diff --git a/arkindex/process/models.py b/arkindex/process/models.py
index 4dec68a471..22127f4062 100644
--- a/arkindex/process/models.py
+++ b/arkindex/process/models.py
@@ -802,7 +802,7 @@ class WorkerVersion(models.Model):
def is_init_elements(self):
return (
- self.id == WorkerVersion.objects.get_by_feature(ArkindexFeature.InitElements).id
+ self.feature == ArkindexFeature.InitElements
# Make detection of the init task in case its version changes
or self.worker.slug.startswith("init-elements")
or self.worker.slug.startswith("init_elements")
@@ -926,7 +926,8 @@ class WorkerRun(models.Model):
.select_related("version__worker")
# The IDs are required so that Django understands that everything is prefetched.
# WorkerVersion.slug will just require the version ID and worker slug
- .only("id", "version_id", "version__worker_id", "version__worker__slug")
+ # The init_elements checks requires the version's feature field
+ .only("id", "version_id", "version__worker_id", "version__worker__slug", "version__feature")
)
parents = [
worker_run.task_slug + slug_suffix
diff --git a/arkindex/process/tasks.py b/arkindex/process/tasks.py
index 7079572ea5..0ccd2865f4 100644
--- a/arkindex/process/tasks.py
+++ b/arkindex/process/tasks.py
@@ -29,7 +29,7 @@ def initialize_activity(process: Process):
worker_run for worker_run in (
process.worker_runs
.select_related("version__worker")
- .only("process_id", "version_id", "configuration_id", "model_version_id", "version__worker__slug")
+ .only("process_id", "version_id", "configuration_id", "model_version_id", "version__worker__slug", "version__feature")
)
# Do not generate worker activities for the elements initialisation task
if not worker_run.version.is_init_elements()
diff --git a/arkindex/process/tests/process/test_start.py b/arkindex/process/tests/process/test_start.py
index aa18dec7ba..32d9e5b2dd 100644
--- a/arkindex/process/tests/process/test_start.py
+++ b/arkindex/process/tests/process/test_start.py
@@ -282,7 +282,7 @@ class TestProcessStart(FixtureAPITestCase):
self.client.force_login(self.user)
- with self.assertNumQueries(15):
+ with self.assertNumQueries(16):
response = self.client.post(
reverse("api:process-start", kwargs={"pk": str(self.workers_process.id)})
)
diff --git a/arkindex/process/tests/test_templates.py b/arkindex/process/tests/test_templates.py
index 2d0c47c3c7..835a361c8a 100644
--- a/arkindex/process/tests/test_templates.py
+++ b/arkindex/process/tests/test_templates.py
@@ -90,7 +90,7 @@ class TestTemplates(FixtureAPITestCase):
def test_create(self):
self.client.force_login(self.user)
- with self.assertNumQueries(9):
+ with self.assertNumQueries(8):
response = self.client.post(
reverse(
"api:create-process-template", kwargs={"pk": str(self.process_template.id)}
@@ -134,7 +134,7 @@ class TestTemplates(FixtureAPITestCase):
(FeatureUsage.Supported, True),
])
- with self.assertNumQueries(9):
+ with self.assertNumQueries(8):
response = self.client.post(
reverse(
"api:create-process-template", kwargs={"pk": str(self.process_template.id)}
@@ -392,7 +392,7 @@ class TestTemplates(FixtureAPITestCase):
def test_apply(self):
self.assertIsNotNone(self.version_2.docker_image_iid)
self.client.force_login(self.user)
- with self.assertNumQueries(11):
+ with self.assertNumQueries(10):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data={"process_id": str(self.process.id)},
@@ -470,7 +470,7 @@ class TestTemplates(FixtureAPITestCase):
parents=[],
)
# Apply a template that has two other worker runs
- with self.assertNumQueries(13):
+ with self.assertNumQueries(12):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
data={"process_id": str(process.id)},
@@ -521,7 +521,7 @@ class TestTemplates(FixtureAPITestCase):
(FeatureUsage.Supported, True),
])
- with self.assertNumQueries(11):
+ with self.assertNumQueries(10):
response = self.client.post(
reverse("api:apply-process-template", kwargs={"pk": str(self.template.id)}),
{"process_id": str(self.process.id)},
diff --git a/arkindex/project/tests/__init__.py b/arkindex/project/tests/__init__.py
index 09261a5413..f1bc9e90ba 100644
--- a/arkindex/project/tests/__init__.py
+++ b/arkindex/project/tests/__init__.py
@@ -13,7 +13,6 @@ from rest_framework.test import APITestCase
from arkindex.documents.models import Corpus
from arkindex.images.models import ImageServer
-from arkindex.process.models import WorkerVersion
from arkindex.users.models import Group, User
@@ -119,10 +118,6 @@ class ArkindexTestMixin:
# Clean content type cache for SQL requests checks consistency
ContentType.objects.clear_cache()
- # `cache_clear` is a function defined by the `functools.lru_cache` decorator
- # on the function itself, not on its return value
- WorkerVersion.objects.get_by_feature.cache_clear()
-
# Clear the local cached properties so that it is re-fetched on each test
# to avoid intermittently changing query counts.
# Using `del` on a cached property that has not been accessed yet can cause an AttributeError.
--
GitLab