From 5825db873db8e62624de763072fd4384824dd005 Mon Sep 17 00:00:00 2001 From: Erwan Rouchet <rouchet@teklia.com> Date: Mon, 14 Nov 2022 14:29:16 +0000 Subject: [PATCH] Remove WorkerRun.worker --- arkindex/process/api.py | 40 ++-- arkindex/process/serializers/imports.py | 36 ++-- arkindex/process/tests/test_workerruns.py | 240 +++++++++++----------- 3 files changed, 162 insertions(+), 154 deletions(-) diff --git a/arkindex/process/api.py b/arkindex/process/api.py index 42bc4ce104..c5ad06e5b6 100644 --- a/arkindex/process/api.py +++ b/arkindex/process/api.py @@ -41,6 +41,7 @@ from arkindex.documents.models import Corpus, Element from arkindex.process.models import ( ActivityState, DataFile, + GitRef, GitRefType, Process, ProcessMode, @@ -1173,51 +1174,56 @@ class WorkerRunList(WorkerACLMixin, ListCreateAPIView): serializer_class = WorkerRunSerializer queryset = WorkerRun.objects.none() - def get_queryset(self): + @cached_property + def process(self): process = get_object_or_404( - Process.objects.filter(corpus_id__isnull=False), + Process.objects.filter(corpus_id__isnull=False).select_related('corpus'), pk=self.kwargs['pk'] ) if not self.has_access(process.corpus, Role.Admin.value): raise PermissionDenied(detail='You do not have an admin access to the corpus of this process.') - return process.worker_runs \ - .select_related('version__worker__type') \ + return process + + def get_queryset(self): + return self.process.worker_runs \ + .select_related( + 'version__worker__repository', + 'version__worker__type', + 'version__revision__repo', + 'model_version__model', + ) \ + .prefetch_related(Prefetch( + 'version__revision__refs', + queryset=GitRef.objects.select_related('repository') + )) \ .order_by('id') def perform_create(self, serializer): - process = get_object_or_404( - Process.objects.filter(corpus_id__isnull=False), - pk=self.kwargs['pk'] - ) - - # User must have admin access to the process project and a contributor access to the worker run - if not self.has_access(process.corpus, Role.Admin.value): - raise PermissionDenied(detail='You do not have an admin access to the corpus of this process.') - try: worker = Worker.objects.get(versions__id=serializer.validated_data['version_id']) except Worker.DoesNotExist: raise ValidationError({'worker_version_id': ['This version does not exist.']}) + # User must have admin access to the process project and a contributor access to the worker run if not self.has_execution_access(worker): raise ValidationError({'worker_version_id': ['You do not have an execution access to this version.']}) - if process.worker_runs.filter(version_id=serializer.validated_data['version_id']).exists(): + if self.process.worker_runs.filter(version_id=serializer.validated_data['version_id']).exists(): raise ValidationError({'worker_version_id': ['A WorkerRun with this version already exists on this process.']}) configuration = serializer.validated_data.pop('configuration_id', None) if configuration and configuration.worker_id != worker.id: raise ValidationError({'configuration_id': ['The configuration must be part of the same worker.']}) - if process.mode != ProcessMode.Workers: + if self.process.mode != ProcessMode.Workers: raise ValidationError({'process': ['Import mode must be Workers']}) - if process.workflow_id is not None: + if self.process.workflow_id is not None: raise ValidationError({'__all__': ["Cannot create a WorkerRun on a Process that has already started"]}) - serializer.save(process=process, configuration=configuration) + serializer.save(process=self.process, configuration=configuration) @extend_schema(tags=['process']) diff --git a/arkindex/process/serializers/imports.py b/arkindex/process/serializers/imports.py index 877e28579b..630b68d78b 100644 --- a/arkindex/process/serializers/imports.py +++ b/arkindex/process/serializers/imports.py @@ -6,11 +6,7 @@ from arkindex.documents.models import Corpus, Element, ElementType from arkindex.documents.serializers.elements import ElementSlimSerializer from arkindex.process.models import ActivityState, DataFile, Process, ProcessMode, WorkerConfiguration, WorkerRun from arkindex.process.serializers.git import RevisionSerializer -from arkindex.process.serializers.workers import ( - WorkerConfigurationSerializer, - WorkerLightSerializer, - WorkerVersionSerializer, -) +from arkindex.process.serializers.workers import WorkerConfigurationSerializer, WorkerVersionSerializer from arkindex.project.mixins import ProcessACLMixin from arkindex.project.serializer_fields import EnumField, LinearRingField from arkindex.training.models import Model, ModelVersion @@ -400,22 +396,29 @@ class WorkerRunSerializer(serializers.ModelSerializer): Serialize a worker run """ parents = serializers.ListField(child=serializers.UUIDField()) + worker_version_id = serializers.UUIDField(source='version_id') - # Serialize worker with its basic information - worker = WorkerLightSerializer(source='version.worker', read_only=True) + worker_version = WorkerVersionSerializer(read_only=True, source='version') + model_version = ModelVersionLightSerializer(read_only=True) - configuration_id = serializers.PrimaryKeyRelatedField(queryset=WorkerConfiguration.objects.all(), required=False, allow_null=True) + + configuration_id = serializers.PrimaryKeyRelatedField( + queryset=WorkerConfiguration.objects.all(), + required=False, + allow_null=True, + style={'base_template': 'input.html'}, + ) class Meta: model = WorkerRun - read_only_fields = ('id', 'worker', 'process_id', 'model_version_id') + read_only_fields = ('id', 'process_id', 'model_version_id') fields = ( 'id', 'parents', 'worker_version_id', + 'worker_version', 'model_version_id', 'process_id', - 'worker', 'model_version', 'configuration_id', ) @@ -425,16 +428,19 @@ class WorkerRunEditSerializer(WorkerRunSerializer): """ Serialize a worker run with only parents as editable field """ - worker_version_id = serializers.UUIDField(read_only=True, source='version_id') - model_version_id = serializers.PrimaryKeyRelatedField(queryset=ModelVersion.objects.all(), required=False, allow_null=True, source='model_version') - - worker_version = WorkerVersionSerializer(read_only=True, source='version') + worker_version_id = serializers.UUIDField(source='version_id', read_only=True) + model_version_id = serializers.PrimaryKeyRelatedField( + queryset=ModelVersion.objects.all(), + required=False, + allow_null=True, + source='model_version', + style={'base_template': 'input.html'}, + ) configuration = WorkerConfigurationSerializer(read_only=True) process = ProcessTrainingSerializer(read_only=True) class Meta(WorkerRunSerializer.Meta): fields = WorkerRunSerializer.Meta.fields + ( - 'worker_version', 'configuration', 'process', ) diff --git a/arkindex/process/tests/test_workerruns.py b/arkindex/process/tests/test_workerruns.py index 239a886195..3efe1df0da 100644 --- a/arkindex/process/tests/test_workerruns.py +++ b/arkindex/process/tests/test_workerruns.py @@ -79,15 +79,34 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertCountEqual(data['results'], [{ 'id': str(self.run_1.id), 'worker_version_id': str(self.version_1.id), + 'worker_version': { + 'configuration': {'test': 42}, + 'docker_image': str(self.version_1.docker_image.id), + 'docker_image_iid': None, + 'docker_image_name': f'my_repo.fake/workers/worker/reco:{self.version_1.id}', + 'gpu_usage': 'disabled', + 'id': str(self.version_1.id), + 'model_usage': False, + 'revision': { + 'author': 'Test user', + 'commit_url': 'http://my_repo.fake/workers/worker/commit/1337', + 'created': self.version_1.revision.created.isoformat().replace('+00:00', 'Z'), + 'hash': '1337', + 'id': str(self.version_1.revision.id), + 'message': 'My w0rk3r', + 'refs': [] + }, + 'state': 'available', + 'worker': { + 'id': str(self.worker_1.id), + 'name': 'Recognizer', + 'slug': 'reco', + 'type': 'recognizer' + } + }, 'process_id': str(self.process_1.id), 'parents': [], 'model_version_id': None, - 'worker': { - 'id': str(self.worker_1.id), - 'name': self.worker_1.name, - 'type': self.worker_1.type.slug, - 'slug': self.worker_1.slug, - }, 'model_version': None, 'configuration_id': None, }]) @@ -183,26 +202,45 @@ class TestWorkerRuns(FixtureAPITestCase): def test_runs_post_create_worker_run(self): self.client.force_login(self.user) - with self.assertNumQueries(17): + with self.assertNumQueries(19): response = self.client.post( reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}), data={'worker_version_id': str(self.version_1.id), 'parents': []}, format='json' ) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) data = response.json() pk = data.pop('id') self.assertNotEqual(pk, self.run_1.id) self.assertDictEqual(data, { 'worker_version_id': str(self.version_1.id), + 'worker_version': { + 'configuration': {'test': 42}, + 'docker_image': str(self.version_1.docker_image.id), + 'docker_image_iid': None, + 'docker_image_name': f'my_repo.fake/workers/worker/reco:{self.version_1.id}', + 'gpu_usage': 'disabled', + 'id': str(self.version_1.id), + 'model_usage': False, + 'revision': { + 'author': 'Test user', + 'commit_url': 'http://my_repo.fake/workers/worker/commit/1337', + 'created': self.version_1.revision.created.isoformat().replace('+00:00', 'Z'), + 'hash': '1337', + 'id': str(self.version_1.revision.id), + 'message': 'My w0rk3r', + 'refs': [] + }, + 'state': 'available', + 'worker': { + 'id': str(self.worker_1.id), + 'name': 'Recognizer', + 'slug': 'reco', + 'type': 'recognizer' + } + }, 'process_id': str(self.process_2.id), 'parents': [], 'model_version_id': None, - 'worker': { - 'id': str(self.worker_1.id), - 'name': self.worker_1.name, - 'type': self.worker_1.type.slug, - 'slug': self.worker_1.slug, - }, 'model_version': None, 'configuration_id': None, }) @@ -214,7 +252,7 @@ class TestWorkerRuns(FixtureAPITestCase): self.worker_1.memberships.filter(user=self.user).update(level=Role.Guest.value) self.worker_1.repository.memberships.create(user=self.user, level=Role.Contributor.value) self.client.force_login(self.user) - with self.assertNumQueries(18): + with self.assertNumQueries(20): response = self.client.post( reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}), data={'worker_version_id': str(self.version_1.id), 'parents': []}, format='json' @@ -237,7 +275,7 @@ class TestWorkerRuns(FixtureAPITestCase): def test_create_run_configuration(self): self.client.force_login(self.user) - with self.assertNumQueries(18): + with self.assertNumQueries(20): response = self.client.post( reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}), data={ @@ -254,15 +292,34 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertNotEqual(pk, self.run_1.id) self.assertDictEqual(data, { 'worker_version_id': str(self.version_1.id), + 'worker_version': { + 'configuration': {'test': 42}, + 'docker_image': str(self.version_1.docker_image.id), + 'docker_image_iid': None, + 'docker_image_name': f'my_repo.fake/workers/worker/reco:{self.version_1.id}', + 'gpu_usage': 'disabled', + 'id': str(self.version_1.id), + 'model_usage': False, + 'revision': { + 'author': 'Test user', + 'commit_url': 'http://my_repo.fake/workers/worker/commit/1337', + 'created': self.version_1.revision.created.isoformat().replace('+00:00', 'Z'), + 'hash': '1337', + 'id': str(self.version_1.revision.id), + 'message': 'My w0rk3r', + 'refs': [] + }, + 'state': 'available', + 'worker': { + 'id': str(self.worker_1.id), + 'name': 'Recognizer', + 'slug': 'reco', + 'type': 'recognizer' + } + }, 'process_id': str(self.process_2.id), 'parents': [], 'model_version_id': None, - 'worker': { - 'id': str(self.worker_1.id), - 'name': self.worker_1.name, - 'type': self.worker_1.type.slug, - 'slug': self.worker_1.slug, - }, 'model_version': None, 'configuration_id': str(self.configuration_1.id) }) @@ -292,37 +349,61 @@ class TestWorkerRuns(FixtureAPITestCase): author="Teklia Bot" ) git_ref_names = ["main", "develop", "trunk", "master", "patate", "pouet"] + expected_refs = [] for git_ref_name in git_ref_names: - revision.refs.create( + new_ref = revision.refs.create( name=git_ref_name, type=GitRefType.Branch, repository=revision.repo ) + expected_refs.append({'id': str(new_ref.id), 'name': git_ref_name, 'type': 'branch'}) + version = WorkerVersion.objects.create( worker=self.worker_1, revision=revision, configuration={} ) - with self.assertNumQueries(17): + + with self.assertNumQueries(19): response = self.client.post( reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}), data={'worker_version_id': str(version.id), 'parents': []}, format='json' ) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + data = response.json() pk = data.pop('id') self.assertNotEqual(pk, self.run_1.id) self.assertDictEqual(data, { 'worker_version_id': str(version.id), + 'worker_version': { + 'configuration': {}, + 'docker_image': None, + 'docker_image_iid': None, + 'docker_image_name': f'my_repo.fake/workers/worker/reco:{version.id}', + 'gpu_usage': 'disabled', + 'id': str(version.id), + 'model_usage': False, + 'revision': { + 'author': 'Teklia Bot', + 'commit_url': f'http://my_repo.fake/workers/worker/commit/{version.revision.hash}', + 'created': version.revision.created.isoformat().replace('+00:00', 'Z'), + 'hash': version.revision.hash, + 'id': str(version.revision.id), + 'message': 'Fake revision', + 'refs': expected_refs, + }, + 'state': 'created', + 'worker': { + 'id': str(self.worker_1.id), + 'name': 'Recognizer', + 'slug': 'reco', + 'type': 'recognizer' + } + }, 'process_id': str(self.process_2.id), 'parents': [], 'model_version_id': None, - 'worker': { - 'id': str(self.worker_1.id), - 'name': self.worker_1.name, - 'type': self.worker_1.type.slug, - 'slug': self.worker_1.slug, - }, 'model_version': None, 'configuration_id': None }) @@ -366,12 +447,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 42}, 'docker_image': str(self.version_1.docker_image.id), @@ -433,12 +508,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 42}, 'docker_image': str(self.version_1.docker_image.id), @@ -655,7 +724,7 @@ class TestWorkerRuns(FixtureAPITestCase): 'parents': [], }, format='json' ) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.json(), { 'configuration': None, @@ -678,12 +747,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 42}, 'docker_image': str(self.version_1.docker_image.id), @@ -728,7 +791,7 @@ class TestWorkerRuns(FixtureAPITestCase): 'parents': [], }, format='json' ) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.json(), { 'configuration': None, 'configuration_id': None, @@ -750,12 +813,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 42}, 'docker_image': str(self.version_1.docker_image.id), @@ -829,12 +886,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 42}, 'docker_image': str(self.version_1.docker_image.id), @@ -1067,12 +1118,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 'test2'}, 'docker_image': None, @@ -1137,7 +1182,7 @@ class TestWorkerRuns(FixtureAPITestCase): 'parents': [] }, format='json' ) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_200_OK) run.refresh_from_db() self.assertEqual(response.json(), { 'configuration': { @@ -1175,12 +1220,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 'test2'}, 'docker_image': None, @@ -1235,7 +1274,7 @@ class TestWorkerRuns(FixtureAPITestCase): 'parents': [str(run_2.id)], }, format='json' ) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_200_OK) self.run_1.refresh_from_db() self.assertDictEqual(response.json(), { 'configuration': None, @@ -1258,12 +1297,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 42}, 'docker_image': str(self.version_1.docker_image.id), @@ -1365,7 +1398,7 @@ class TestWorkerRuns(FixtureAPITestCase): 'parents': ['12341234-1234-1234-1234-123412341234'], }, format='json' ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), [ f"Can't add or update WorkerRun {self.run_1.id} because parents field isn't properly defined. It can be either because" " one or several UUIDs don't refer to existing WorkerRuns or either because listed WorkerRuns doesn't belong to the" @@ -1407,12 +1440,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 42}, 'docker_image': str(self.version_1.docker_image.id), @@ -1479,12 +1506,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 42}, 'docker_image': str(self.version_1.docker_image.id), @@ -1555,12 +1576,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 42}, 'docker_image': str(self.version_1.docker_image.id), @@ -1753,7 +1768,7 @@ class TestWorkerRuns(FixtureAPITestCase): 'model_version_id': str(self.model_version_1.id), }, format='json' ) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_200_OK) run.refresh_from_db() self.assertEqual(response.json(), { 'configuration': None, @@ -1786,12 +1801,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 'test2'}, 'docker_image': None, @@ -1854,7 +1863,6 @@ class TestWorkerRuns(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_200_OK) run.refresh_from_db() - self.maxDiff = None self.assertEqual(response.json(), { 'configuration': { 'archived': False, @@ -1891,12 +1899,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 'test2'}, 'docker_image': None, @@ -1973,12 +1975,6 @@ class TestWorkerRuns(FixtureAPITestCase): 'workflow': None }, 'process_id': str(self.process_1.id), - 'worker': { - 'id': str(self.worker_1.id), - 'name': 'Recognizer', - 'slug': 'reco', - 'type': 'recognizer' - }, 'worker_version': { 'configuration': {'test': 42}, 'docker_image': str(self.version_1.docker_image.id), -- GitLab