diff --git a/arkindex/process/api.py b/arkindex/process/api.py index 70be4c3f56d0653249b6902fb412cd2a0be2007e..7b0d2cc1f66cf5f897a1ba5ed7926f1408aaa132 100644 --- a/arkindex/process/api.py +++ b/arkindex/process/api.py @@ -73,7 +73,6 @@ from arkindex.process.models import ( WorkerRun, WorkerType, WorkerVersion, - WorkerVersionState, ) from arkindex.process.providers import GitProvider from arkindex.process.serializers.files import DataFileCreateSerializer, DataFileSerializer @@ -1334,20 +1333,19 @@ class WorkerConfigurationRetrieve(WorkerACLMixin, RetrieveUpdateAPIView): @extend_schema(tags=['process']) @extend_schema_view( - get=extend_schema(description=( - 'List worker runs for a given process ID.\n\n' - 'Requires an **admin** access to the process project.' - )), - post=extend_schema(description=( - 'Create a worker run for a given process ID.\n\n' - 'Requires an **admin** access to the process project and a **contributor** access to the worker.' - )), + get=extend_schema(description=dedent(""" + List worker runs for a given process ID. + + Requires an **admin** access on the process. + """)), + post=extend_schema(description=dedent(""" + Create a worker run for a given process ID. + + Requires an **admin** access to the process and a **contributor** access to the worker. + WorkerRuns may only be created on `dataset` and `workers` processes that have not already started. + """)), ) -class WorkerRunList(WorkerACLMixin, ListCreateAPIView): - """ - List worker runs for a given process UUID. - Create a run on a process. - """ +class WorkerRunList(ProcessACLMixin, ListCreateAPIView): permission_classes = (IsVerified, ) serializer_class = WorkerRunSerializer queryset = WorkerRun.objects.none() @@ -1357,14 +1355,15 @@ class WorkerRunList(WorkerACLMixin, ListCreateAPIView): process = get_object_or_404( Process .objects - .filter(corpus_id__isnull=False) + .exclude(corpus_id=None) .select_related('corpus') .annotate(has_tasks=Exists(Task.objects.filter(process=OuterRef('pk')))), 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.') + access_level = self.process_access_level(process) + if access_level is None or access_level < Role.Admin.value: + raise PermissionDenied(detail='You do not have an admin access to this process.') return process @@ -1382,67 +1381,52 @@ class WorkerRunList(WorkerACLMixin, ListCreateAPIView): )) \ .order_by('id') - def perform_create(self, serializer): - # TODO: Aggregate errors using a defaultdict(list) and send them all at once instead of raising early, per best practices - # https://redmine.teklia.com/projects/arkindex/wiki/Arkindex_API_best_practices#Error-responses - try: - version = ( - WorkerVersion.objects - # Required for the WorkerACLMixin - .select_related('worker__repository') - .only('id', 'state', 'worker__id', 'worker__public', 'worker__repository__id') - .get(id=serializer.validated_data['version_id']) - ) - except WorkerVersion.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(version.worker): - raise ValidationError({'worker_version_id': ['You do not have an execution access to this version.']}) - - 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.']}) - - if version.state != WorkerVersionState.Available: - raise ValidationError({'worker_version_id': ['This worker version is not in an Available state.']}) - - configuration = serializer.validated_data.pop('configuration_id', None) - if configuration and configuration.worker_id != version.worker.id: - raise ValidationError({'configuration_id': ['The configuration must be part of the same worker.']}) - - if self.process.mode not in (ProcessMode.Workers, ProcessMode.Dataset): - raise ValidationError({'process': ['WorkerRuns can only be created on Workers or Dataset processes.']}) - - if self.process.has_tasks: - raise ValidationError({'__all__': ["Cannot create a WorkerRun on a Process that has already started"]}) - - serializer.save(process=self.process, configuration=configuration) - - # Since we only allow creating a WorkerRun on a process without any task, we know the last run is None, - # without having to make any extra SQL queries to serialize it. - self.process.last_run = None + def get_serializer_context(self): + context = super().get_serializer_context() + context['process'] = self.process + return context @extend_schema(tags=['process']) @extend_schema_view( get=extend_schema( operation_id='RetrieveWorkerRun', - description='Retrieve details of a worker run.\n\nRequires an **admin** access to the process project.' + description=dedent(""" + Retrieve details of a worker run. + + Requires an **admin** access on the process. + WorkerRuns may only be retrieved on `dataset` and `workers` processes, or on the `local` process of the authenticated user. + """), ), patch=extend_schema( operation_id='PartialUpdateWorkerRun', - description='Partially update a worker run.\n\nRequires an **admin** access to the process project.' + description=dedent(""" + Partially update a worker run. + + Requires an **admin** access on the process. + WorkerRuns may only be updated on `dataset` and `workers` processes that have not already started. + """), ), put=extend_schema( operation_id='UpdateWorkerRun', - description='Update dependencies of a worker run.\n\nRequires an **admin** access to the process project.' + description=dedent(""" + Update a worker run. + + Requires an **admin** access on the process. + WorkerRuns may only be updated on `dataset` and `workers` processes that have not already started. + """), ), delete=extend_schema( operation_id='DestroyWorkerRun', - description='Delete a worker run.\n\nRequires an **admin** access to the process project.' + description=dedent(""" + Delete a worker run. + + Requires an **admin** access on the process. + WorkerRuns may only be deletedf from `dataset` and `workers` processes that have not already started. + """), ), ) -class WorkerRunDetails(CorpusACLMixin, RetrieveUpdateDestroyAPIView): +class WorkerRunDetails(ProcessACLMixin, RetrieveUpdateDestroyAPIView): """ Retrieve or edit a worker run i.e. update its dependencies. Requires an admin access to the process project. @@ -1454,11 +1438,14 @@ class WorkerRunDetails(CorpusACLMixin, RetrieveUpdateDestroyAPIView): if not hasattr(self, '_worker_run'): self._worker_run = super().get_object() - # To avoid an extra unnecessary query for the process' last run, we set it here. - # We will only serialize the process with its state when the query is successful, - # and we only allow editing WorkerRuns on processes that have no tasks at all, - # therefore the last run is always None. - self._worker_run.process.last_run = None + # Require admin access to the process, except for the user's own local process + if self._worker_run.process.mode != ProcessMode.Local or self._worker_run.process.creator != self.request.user: + access_level = self.process_access_level(self._worker_run.process) + if access_level is None or access_level < Role.Admin.value: + raise PermissionDenied(detail='You do not have an admin access to this process.') + + # Django does not support annotations on nested objects, so we apply it manually + self._worker_run.process.has_tasks = self._worker_run.process_has_tasks return self._worker_run @@ -1481,41 +1468,28 @@ class WorkerRunDetails(CorpusACLMixin, RetrieveUpdateDestroyAPIView): 'configuration', 'model_version__model', ) - .prefetch_related('version__revision__refs') + .prefetch_related(Prefetch( + 'version__revision__refs', + queryset=GitRef.objects.select_related('repository') + )) ) return queryset - def get_serializer_context(self): - context = super().get_serializer_context() - if 'model_version_id' in self.request.data: - context['model_usage'] = self.get_object().version.model_usage - return context - - def check_object_permissions(self, request, worker_run): - if worker_run.process.corpus_id and not self.has_admin_access(worker_run.process.corpus): - raise PermissionDenied(detail='You do not have an admin access to the process project.') - - # Updating a worker run is not possible once the process is started - if request.method not in permissions.SAFE_METHODS: - if worker_run.process_has_tasks: - raise ValidationError({'__all__': ["Cannot update a WorkerRun on a Process that has already started"]}) - if worker_run.process.mode == ProcessMode.Local: - raise ValidationError({'__all__': ['Cannot update a WorkerRun on a local process']}) - - super().check_object_permissions(request, worker_run) - def perform_destroy(self, instance): + # No serializer is used when destroying, so we validate the process mode here instead. + errors = [] + if instance.process.mode not in (ProcessMode.Workers, ProcessMode.Dataset): + errors.append('WorkerRuns can only be deleted from Workers or Dataset processes.') + if instance.process.has_tasks: + errors.append('WorkerRuns cannot be deleted from a process that has already started.') + if errors: + raise ValidationError(errors) + # Remove references to this instance from its children parents field instance.process.worker_runs.filter(parents__contains=[instance.id]).update(parents=ArrayRemove('parents', instance.id)) - return super().perform_destroy(instance) - def perform_update(self, serializer): - worker = serializer.instance.version.worker - configuration = serializer.validated_data.get('configuration_id', None) - if configuration and configuration.worker_id != worker.id: - raise ValidationError({'configuration_id': ['The configuration must be part of the same worker.']}) - super().perform_update(serializer) + return super().perform_destroy(instance) @extend_schema_view(post=extend_schema( diff --git a/arkindex/process/migrations/0014_workerrun_multiple_model_configs.py b/arkindex/process/migrations/0014_workerrun_multiple_model_configs.py new file mode 100644 index 0000000000000000000000000000000000000000..596e6fbfd9c4e03a6af9aece4f14cdd2557faecd --- /dev/null +++ b/arkindex/process/migrations/0014_workerrun_multiple_model_configs.py @@ -0,0 +1,49 @@ +# Generated by Django 4.1.7 on 2023-08-08 13:01 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('process', '0013_processdataset_unique_process_dataset'), + ] + + operations = [ + migrations.AlterUniqueTogether( + name='workerrun', + unique_together=set(), + ), + migrations.AddConstraint( + model_name='workerrun', + constraint=models.UniqueConstraint( + condition=models.Q(configuration__isnull=True, model_version__isnull=True), + fields=('process', 'version'), + name='unique_workerrun', + ), + ), + migrations.AddConstraint( + model_name='workerrun', + constraint=models.UniqueConstraint( + condition=models.Q(configuration__isnull=True, model_version__isnull=False), + fields=('process', 'version', 'model_version'), + name='unique_workerrun_model_version', + ), + ), + migrations.AddConstraint( + model_name='workerrun', + constraint=models.UniqueConstraint( + condition=models.Q(configuration__isnull=False, model_version__isnull=True), + fields=('process', 'version', 'configuration'), + name='unique_workerrun_configuration', + ), + ), + migrations.AddConstraint( + model_name='workerrun', + constraint=models.UniqueConstraint( + condition=models.Q(configuration__isnull=False, model_version__isnull=False), + fields=('process', 'version', 'model_version', 'configuration'), + name='unique_workerrun_model_version_and_configuration', + ), + ), + ] diff --git a/arkindex/process/models.py b/arkindex/process/models.py index a7a3e3f062cd2a7b1924a153f8347de1e43f8ba0..a7a0356cba0250c99725fe5b6efa08f25a191a07 100644 --- a/arkindex/process/models.py +++ b/arkindex/process/models.py @@ -1186,7 +1186,28 @@ class WorkerRun(models.Model): summary = models.TextField() class Meta: - unique_together = (('version', 'process'),) + constraints = [ + models.UniqueConstraint( + fields=['process', 'version'], + condition=Q(model_version__isnull=True, configuration__isnull=True), + name='unique_workerrun', + ), + models.UniqueConstraint( + fields=['process', 'version', 'model_version'], + condition=Q(model_version__isnull=False, configuration__isnull=True), + name='unique_workerrun_model_version', + ), + models.UniqueConstraint( + fields=['process', 'version', 'configuration'], + condition=Q(model_version__isnull=True, configuration__isnull=False), + name='unique_workerrun_configuration', + ), + models.UniqueConstraint( + fields=['process', 'version', 'model_version', 'configuration'], + condition=Q(model_version__isnull=False, configuration__isnull=False), + name='unique_workerrun_model_version_and_configuration', + ), + ] def build_task(self, process, task_name, env, import_task_name, elements_path, run=0, chunk=None, workflow_runs=None): ''' diff --git a/arkindex/process/serializers/worker_runs.py b/arkindex/process/serializers/worker_runs.py index 690bd9b3c4e051b60650e0bf9e2a8b0debb3a4ca..6c34b0474bcc290fd99c4636d1dcd3dd71778a34 100644 --- a/arkindex/process/serializers/worker_runs.py +++ b/arkindex/process/serializers/worker_runs.py @@ -1,9 +1,20 @@ +from collections import defaultdict + +from django.db.models import Prefetch from rest_framework import serializers from rest_framework.exceptions import ValidationError -from arkindex.process.models import WorkerConfiguration, WorkerRun +from arkindex.process.models import ( + GitRef, + ProcessMode, + WorkerConfiguration, + WorkerRun, + WorkerVersion, + WorkerVersionState, +) from arkindex.process.serializers.imports import ProcessTrainingSerializer from arkindex.process.serializers.workers import WorkerConfigurationSerializer, WorkerVersionSerializer +from arkindex.project.mixins import WorkerACLMixin from arkindex.training.models import ModelVersion, ModelVersionState from arkindex.training.serializers import ModelVersionLightSerializer from arkindex.users.models import Role @@ -14,62 +25,75 @@ from arkindex.users.utils import get_max_level # do serialize all the related information on WorkerRun serializers. -class WorkerRunSerializer(serializers.ModelSerializer): - """ - Serialize a worker run for creation. - Worker version and parents are required, configuration is optional. - """ - parents = serializers.ListField(child=serializers.UUIDField()) +class WorkerRunSerializer(WorkerACLMixin, serializers.ModelSerializer): + + parents = serializers.ListField( + child=serializers.UUIDField(), + default=list, + help_text='UUIDs of the WorkerRuns that this WorkerRun depends on. ' + 'The WorkerRuns must be on the same process, and the dependencies cannot create a cycle.', + ) - worker_version_id = serializers.UUIDField(write_only=True, source='version_id') worker_version = WorkerVersionSerializer(read_only=True, source='version') + worker_version_id = serializers.PrimaryKeyRelatedField( + write_only=True, + source='version', + queryset=( + WorkerVersion + .objects + .select_related( + # Required for the WorkerACLMixin + 'worker__repository', + # Used by the WorkerVersionSerializer + 'worker__type', + 'revision__repo', + ) + .prefetch_related( + # Used by the WorkerVersionSerializer + Prefetch( + 'revision__refs', + queryset=GitRef.objects.select_related('repository') + ) + ) + ), + help_text='UUID of the WorkerVersion for this WorkerRun. ' + 'Only WorkerVersions in an `available` state may be set. ' + 'Contributor access to the worker is required.', + ) model_version = ModelVersionLightSerializer(read_only=True) - - configuration = WorkerConfigurationSerializer(read_only=True) - process = ProcessTrainingSerializer(read_only=True) - configuration_id = serializers.PrimaryKeyRelatedField( - queryset=WorkerConfiguration.objects.all(), + model_version_id = serializers.PrimaryKeyRelatedField( + queryset=ModelVersion.objects.all().select_related('model'), required=False, allow_null=True, write_only=True, + source='model_version', style={'base_template': 'input.html'}, + help_text='UUID of the ModelVersion for this WorkerRun, or `null` if none is set. ' + 'Only ModelVersions in an `available` state may be set. ' + 'Contributor access to the model is required.', ) - class Meta: - model = WorkerRun - fields = ( - 'id', - 'parents', - 'worker_version_id', - 'worker_version', - 'process', - 'configuration_id', - 'configuration', - 'model_version', - ) - - -class WorkerRunEditSerializer(WorkerRunSerializer): - """ - Serialize a worker run for edition. - Process and worker version cannot be edited. - Parents, model version and configuration can be edited. - """ - model_version_id = serializers.PrimaryKeyRelatedField( - queryset=ModelVersion.objects.all().select_related('model'), + configuration = WorkerConfigurationSerializer(read_only=True) + configuration_id = serializers.PrimaryKeyRelatedField( + queryset=WorkerConfiguration.objects.all(), required=False, allow_null=True, write_only=True, - source='model_version', style={'base_template': 'input.html'}, + source='configuration', + help_text='UUID of the WorkerConfiguration for this WorkerRun, or `null` if none is set. ' + "Only a configuration of the WorkerVersion's worker may be set.", ) + process = ProcessTrainingSerializer(read_only=True) + class Meta: model = WorkerRun fields = ( 'id', 'parents', + 'worker_version_id', 'worker_version', 'process', 'configuration_id', @@ -78,23 +102,70 @@ class WorkerRunEditSerializer(WorkerRunSerializer): 'model_version', ) - def validate_model_version_id(self, model_version): - model_usage = self.context.get('model_usage') - if not model_usage: - raise ValidationError("This worker version does not support model usage.") + def validate(self, data): + data = super().validate(data) + errors = defaultdict(list) + worker_version, model_version, configuration = None, None, None + + if self.instance: + process = self.instance.process + worker_version = self.instance.version + model_version = self.instance.model_version + configuration = self.instance.configuration + else: + data['process'] = process = self.context['process'] + + worker_version = data.get('version', worker_version) + model_version = data.get('model_version', model_version) + configuration = data.get('configuration', configuration) + + if process.mode not in (ProcessMode.Workers, ProcessMode.Dataset): + errors['process_id'].append('WorkerRuns can only be created or updated on Workers or Dataset processes.') + if process.has_tasks: + errors['process_id'].append('WorkerRuns cannot be added or updated on processes that have already started.') + + # The worker version is not editable on existing WorkerRun instances, so we only validate when creating a new one + if not self.instance: + # Set self.request to please the WorkerACLMixin + self.request = self.context['request'] + if not self.has_execution_access(worker_version.worker): + errors['worker_version_id'].append('You do not have an execution access to this worker.') + elif worker_version.state != WorkerVersionState.Available: + errors['worker_version_id'].append('This WorkerVersion is not in an Available state.') + + if configuration and configuration.worker_id != worker_version.worker.id: + errors['configuration_id'].append('The configuration must be part of the same worker.') + + if model_version: + # We cannot use both the WorkerACLMixin and the TrainingModelMixin at once! + access_level = get_max_level(self.context["request"].user, model_version.model) + if not access_level or access_level < Role.Contributor.value: + errors['model_version_id'].append('You do not have contributor access to this model.') + elif model_version.state != ModelVersionState.Available: + errors['model_version_id'].append('This ModelVersion is not in an Available state.') + + if not worker_version.model_usage: + errors['model_version_id'].append('This worker version does not support models.') + + existing_worker_run = process.worker_runs.filter(version=worker_version, model_version_id=model_version, configuration=configuration) + if self.instance: + # The current worker run cannot be a duplicate of itself! + existing_worker_run = existing_worker_run.exclude(id=self.instance.id) + if existing_worker_run.exists(): + errors['__all__'].append('A WorkerRun already exists on this process with the selected worker version, model version and configuration.') + + if errors: + raise ValidationError(errors) + + # Since we only allow creating a WorkerRun on a process without any task, we know the last run is None, + # without having to make any extra SQL queries to serialize it in the response. + process.last_run = None - if model_version.state != ModelVersionState.Available: - raise ValidationError("This model version is not in an available state.") + return data - # Check access rights on model version - access_level = get_max_level(self.context["request"].user, model_version.model) - if not access_level or access_level < Role.Contributor.value: - raise ValidationError('You do not have access to this model version.') - return model_version +class WorkerRunEditSerializer(WorkerRunSerializer): - def validate(self, data): - # Store configuration if provided - if 'configuration_id' in data: - data['configuration'] = data['configuration_id'] - return data + class Meta(WorkerRunSerializer.Meta): + # Same as WorkerRunSerializer, but the worker_version_id cannot be edited + fields = tuple(set(WorkerRunSerializer.Meta.fields) - {'worker_version_id'}) diff --git a/arkindex/process/tests/test_workerruns.py b/arkindex/process/tests/test_workerruns.py index d98a7d8a0e4f4023a210bc348fdf185b366885cc..23547ea961c06ba34a4b701ecd4061d0065b84ed 100644 --- a/arkindex/process/tests/test_workerruns.py +++ b/arkindex/process/tests/test_workerruns.py @@ -45,22 +45,26 @@ class TestWorkerRuns(FixtureAPITestCase): cls.model_1.memberships.create(user=cls.user, level=Role.Contributor.value) cls.model_version_1 = ModelVersion.objects.create(model=cls.model_1, state=ModelVersionState.Available, size=8, hash='aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', archive_hash='bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb') - def test_runs_list_requires_login(self): - response = self.client.get(reverse('api:worker-run-list', kwargs={'pk': str(self.process_1.id)})) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + def test_list_requires_login(self): + with self.assertNumQueries(0): + response = self.client.get(reverse('api:worker-run-list', kwargs={'pk': str(self.process_1.id)})) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_runs_list_no_execution_right(self): + def test_list_no_execution_right(self): """ Worker runs attached to a process can be listed even if the user has no execution rights to workers This is due to the fact a user can see a process running on a corpus they have access """ self.worker_1.memberships.all().delete() self.client.force_login(self.user) - response = self.client.get(reverse('api:worker-run-list', kwargs={'pk': str(self.process_1.id)})) - self.assertEqual(response.status_code, status.HTTP_200_OK) + + with self.assertNumQueries(10): + response = self.client.get(reverse('api:worker-run-list', kwargs={'pk': str(self.process_1.id)})) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json()['count'], 1) - def test_runs_list(self): + def test_list(self): self.client.force_login(self.user) with self.assertNumQueries(10): @@ -113,79 +117,129 @@ class TestWorkerRuns(FixtureAPITestCase): }, }]) - def test_runs_list_filter_process(self): + def test_list_filter_process(self): run_2 = self.process_2.worker_runs.create( version=self.version_1, parents=[], ) - self.client.force_login(self.user) - response = self.client.get(reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)})) - self.assertEqual(response.status_code, status.HTTP_200_OK) + + with self.assertNumQueries(10): + response = self.client.get(reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)})) + self.assertEqual(response.status_code, status.HTTP_200_OK) + data = response.json() self.assertEqual(data['count'], 1) self.assertEqual(data['results'][0]['id'], str(run_2.id)) - def test_runs_post_requires_login(self): - 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_403_FORBIDDEN) + def test_create_requires_login(self): + with self.assertNumQueries(0): + 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_403_FORBIDDEN) + self.assertDictEqual(response.json(), {'detail': 'Authentication credentials were not provided.'}) - def test_runs_post_no_execution_right(self): + def test_create_no_execution_right(self): """ An execution access on the target worker version is required to create a worker run """ self.worker_1.memberships.update(level=Role.Guest.value) self.client.force_login(self.user) - 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_400_BAD_REQUEST) - self.assertEqual(response.json(), {'worker_version_id': ['You do not have an execution access to this version.']}) - def test_runs_post_invalid_version(self): + with self.assertNumQueries(12): + 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_400_BAD_REQUEST) + + self.assertEqual(response.json(), {'worker_version_id': ['You do not have an execution access to this worker.']}) + + def test_create_invalid_version(self): self.client.force_login(self.user) - response = self.client.post( - reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}), - data={'worker_version_id': uuid.uuid4(), 'parents': []}, format='json' - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'worker_version_id': ['This version does not exist.']}) + version_id = uuid.uuid4() - def test_create_run_unique(self): + with self.assertNumQueries(7): + response = self.client.post( + reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}), + data={'worker_version_id': version_id}, + format='json', + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + self.assertEqual(response.json(), { + 'worker_version_id': [f'Invalid pk "{version_id}" - object does not exist.'], + }) + + def test_create_unique(self): self.client.force_login(self.user) - response = self.client.post( - reverse('api:worker-run-list', kwargs={'pk': str(self.process_1.id)}), - data={'worker_version_id': str(self.version_1.id), 'parents': []}, format='json' - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'worker_version_id': ['A WorkerRun with this version already exists on this process.']}) + self.version_1.model_usage = True + self.version_1.save() + cases = [ + (None, None), + (None, self.configuration_1), + (self.model_version_1, None), + (self.model_version_1, self.configuration_1), + ] + for model_version, configuration in cases: + with self.subTest(model_version=model_version, configuration=configuration): + self.run_1.model_version = model_version + self.run_1.configuration = configuration + self.run_1.save() + + # Having a model version set adds three queries, having a configuration adds one + query_count = 11 + bool(model_version) * 3 + bool(configuration) + + with self.assertNumQueries(query_count): + response = self.client.post( + reverse('api:worker-run-list', kwargs={'pk': str(self.process_1.id)}), + data={ + 'worker_version_id': str(self.version_1.id), + 'model_version_id': str(model_version.id) if model_version else None, + 'configuration_id': str(configuration.id) if configuration else None, + }, + format='json', + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + self.assertEqual(response.json(), { + '__all__': ['A WorkerRun already exists on this process with the selected worker version, model version and configuration.'], + }) - def test_create_run_unavailable_version(self): + def test_create_unavailable_version(self): self.version_1.state = WorkerVersionState.Error self.version_1.save() self.client.force_login(self.user) - 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_400_BAD_REQUEST) - self.assertEqual(response.json(), {'worker_version_id': ['This worker version is not in an Available state.']}) - def test_runs_post_invalid_process_id(self): + with self.assertNumQueries(11): + 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_400_BAD_REQUEST) + + self.assertEqual(response.json(), {'worker_version_id': ['This WorkerVersion is not in an Available state.']}) + + def test_create_invalid_process_id(self): self.client.force_login(self.user) - response = self.client.post( - reverse('api:worker-run-list', kwargs={'pk': uuid.uuid4()}), - data={'worker_version_id': str(self.version_1.id), 'parents': []}, format='json' - ) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + with self.assertNumQueries(3): + response = self.client.post( + reverse('api:worker-run-list', kwargs={'pk': uuid.uuid4()}), + data={'worker_version_id': str(self.version_1.id), 'parents': []}, + format='json', + ) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.json(), {'detail': 'Not found.'}) - def test_runs_post_invalid_process_mode(self): + def test_create_invalid_process_mode(self): self.client.force_login(self.user) for mode in set(ProcessMode) - {ProcessMode.Workers, ProcessMode.Dataset, ProcessMode.Local, ProcessMode.Repository}: @@ -193,19 +247,19 @@ class TestWorkerRuns(FixtureAPITestCase): self.process_2.mode = mode self.process_2.save() - with self.assertNumQueries(10): + with self.assertNumQueries(11): response = self.client.post( reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}), - {'worker_version_id': str(self.version_1.id), 'parents': []}, + {'worker_version_id': str(self.version_1.id)}, format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), { - 'process': ['WorkerRuns can only be created on Workers or Dataset processes.'], + 'process_id': ['WorkerRuns can only be created or updated on Workers or Dataset processes.'], }) - def test_runs_post_non_corpus_process_mode(self): + def test_create_non_corpus_process_mode(self): process = self.user.processes.get(mode=ProcessMode.Local) self.client.force_login(self.user) @@ -214,7 +268,7 @@ class TestWorkerRuns(FixtureAPITestCase): process.mode = mode process.save() - with self.assertNumQueries(7): + with self.assertNumQueries(3): response = self.client.post( reverse('api:worker-run-list', kwargs={'pk': str(process.id)}), {'worker_version_id': str(self.version_1.id), 'parents': []}, @@ -222,7 +276,7 @@ class TestWorkerRuns(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_runs_post_process_already_started(self): + def test_create_process_already_started(self): process = self.corpus.processes.create( creator=self.user, mode=ProcessMode.Workers, @@ -230,16 +284,20 @@ class TestWorkerRuns(FixtureAPITestCase): process.start() self.client.force_login(self.user) - response = self.client.post( - reverse('api:worker-run-list', kwargs={'pk': str(process.id)}), - data={'worker_version_id': str(self.version_1.id), 'parents': []}, format='json' - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'__all__': [ - 'Cannot create a WorkerRun on a Process that has already started' - ]}) - def test_runs_post_create_worker_run(self): + with self.assertNumQueries(11): + response = self.client.post( + reverse('api:worker-run-list', kwargs={'pk': str(process.id)}), + data={'worker_version_id': str(self.version_1.id), 'parents': []}, + format='json', + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + self.assertEqual(response.json(), { + 'process_id': ['WorkerRuns cannot be added or updated on processes that have already started.'], + }) + + def test_create(self): self.client.force_login(self.user) for mode in (ProcessMode.Workers, ProcessMode.Dataset): @@ -249,7 +307,7 @@ class TestWorkerRuns(FixtureAPITestCase): self.process_2.mode = mode self.process_2.save() - with self.assertNumQueries(19): + with self.assertNumQueries(13): response = self.client.post( reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}), {'worker_version_id': str(self.version_1.id), 'parents': []}, @@ -307,34 +365,39 @@ class TestWorkerRuns(FixtureAPITestCase): # Check generated summary, this supposes that self.version_1.revision does not have any git refs self.assertEqual(run.summary, f"Worker {self.worker_1.name} @ {str(self.version_1.id)[0:6]}") - def test_runs_post_via_repository_right(self): + def test_create_via_repository_right(self): 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(19): + + with self.assertNumQueries(13): 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' + 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) - def test_runs_post_empty(self): + def test_create_empty(self): """ - Parents and worker_version_id are required to create a worker run + The worker_version_id is required to create a worker run """ self.client.force_login(self.user) - response = self.client.post( - reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}) - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + with self.assertNumQueries(6): + response = self.client.post( + reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}) + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertDictEqual(response.json(), { - 'parents': ['This field is required.'], - 'worker_version_id': ['This field is required.'] + 'worker_version_id': ['This field is required.'], }) - def test_create_run_configuration(self): + def test_create_configuration(self): self.client.force_login(self.user) - with self.assertNumQueries(20): + + with self.assertNumQueries(14): response = self.client.post( reverse('api:worker-run-list', kwargs={'pk': str(self.process_2.id)}), data={ @@ -342,7 +405,7 @@ class TestWorkerRuns(FixtureAPITestCase): 'parents': [], 'configuration_id': str(self.configuration_1.id) }, - format='json' + format='json', ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) @@ -401,16 +464,20 @@ class TestWorkerRuns(FixtureAPITestCase): # Check generated summary, this supposes that self.version_1.revision does not have any git refs self.assertEqual(run.summary, f"Worker {self.worker_1.name} @ {str(self.version_1.id)[0:6]} using configuration '{self.configuration_1.name}'") - def test_create_run_invalid_configuration(self): + def test_create_invalid_configuration(self): self.client.force_login(self.user) - 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': [], 'configuration_id': str(self.configuration_2.id)}, format='json' - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + with self.assertNumQueries(12): + 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': [], 'configuration_id': str(self.configuration_2.id)}, + format='json', + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertDictEqual(response.json(), {'configuration_id': ['The configuration must be part of the same worker.']}) - def test_runs_worker_run_summary_with_git_refs(self): + def test_list_summary_with_git_refs(self): """ Create a worker run with a worker version related to a revision tagged with a GitRef """ @@ -440,10 +507,11 @@ class TestWorkerRuns(FixtureAPITestCase): docker_image_id=self.version_1.docker_image_id, ) - with self.assertNumQueries(19): + with self.assertNumQueries(13): 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' + data={'worker_version_id': str(version.id), 'parents': []}, + format='json', ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) @@ -497,21 +565,24 @@ class TestWorkerRuns(FixtureAPITestCase): # Check generated summary self.assertEqual(run.summary, f"Worker {self.worker_1.name} @ main, develop, trunk, master, patate, pouet") - def test_retrieve_run_requires_login(self): - response = self.client.get(reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)})) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + def test_retrieve_requires_login(self): + with self.assertNumQueries(0): + response = self.client.get(reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)})) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_retrieve_run_no_worker_execution_right(self): + def test_retrieve_no_worker_execution_right(self): """ A user can retrieve any worker run if they have an admin access to the process project """ self.worker_1.memberships.update(level=Role.Guest.value) self.client.force_login(self.user) - with self.assertNumQueries(8): + + with self.assertNumQueries(9): response = self.client.get( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) ) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(response.json(), { 'id': str(self.run_1.id), 'configuration': None, @@ -557,18 +628,22 @@ class TestWorkerRuns(FixtureAPITestCase): }, }) - def test_retrieve_run_invalid_id(self): + def test_retrieve_invalid_id(self): self.client.force_login(self.user) - response = self.client.get( - reverse('api:worker-run-details', kwargs={'pk': '12341234-1234-1234-1234-123412341234'}) - ) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_retrieve_run(self): + with self.assertNumQueries(3): + response = self.client.get( + reverse('api:worker-run-details', kwargs={'pk': '12341234-1234-1234-1234-123412341234'}) + ) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_retrieve(self): self.client.force_login(self.user) - with self.assertNumQueries(8): + + with self.assertNumQueries(9): response = self.client.get(reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)})) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(response.json(), { 'id': str(self.run_1.id), 'configuration': None, @@ -657,9 +732,10 @@ class TestWorkerRuns(FixtureAPITestCase): # Check that the gitrefs are retrieved with RetrieveWorkerRun self.client.force_login(self.user) - with self.assertNumQueries(8): + with self.assertNumQueries(9): response = self.client.get(reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)})) self.assertEqual(response.status_code, status.HTTP_200_OK) + version_revision = response.json()['worker_version']['revision'] refs = version_revision.pop('refs') # Only asserting on the name and type of the refs since they were created in bulk using @@ -694,7 +770,7 @@ class TestWorkerRuns(FixtureAPITestCase): } ]) - def test_retrieve_run_local(self): + def test_retrieve_local(self): """ A user can retrieve a run on their own local process """ @@ -702,7 +778,7 @@ class TestWorkerRuns(FixtureAPITestCase): run = local_process.worker_runs.create(version=self.version_1, parents=[]) self.client.force_login(self.user) - with self.assertNumQueries(5): + with self.assertNumQueries(7): response = self.client.get( reverse('api:worker-run-details', kwargs={'pk': str(run.id)}), ) @@ -753,7 +829,7 @@ class TestWorkerRuns(FixtureAPITestCase): }, }) - def test_retrieve_run_local_only_current_user(self): + def test_retrieve_local_only_current_user(self): """ A user cannot retrieve a run on another user's local process """ @@ -766,20 +842,18 @@ class TestWorkerRuns(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_update_run_requires_id_and_parents(self): + def test_update_requires_nothing(self): self.client.force_login(self.user) - with self.assertNumQueries(8): + + with self.assertNumQueries(11): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), - data={ - 'configuration_id': str(self.configuration_1.id), - }, format='json' + data={}, + format='json', ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - - self.assertDictEqual(response.json(), {'parents': ['This field is required.']}) + self.assertEqual(response.status_code, status.HTTP_200_OK) - def test_update_run_requires_login(self): + def test_update_requires_login(self): rev_2 = self.repo.revisions.create( hash='2', message='commit message', @@ -800,11 +874,12 @@ class TestWorkerRuns(FixtureAPITestCase): reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'parents': [str(run_2.id)], - }, format='json' + }, + format='json', ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_update_run_no_project_admin_right(self): + def test_update_no_project_admin_right(self): """ A user cannot update a worker run if he has no admin access on its process project """ @@ -820,9 +895,9 @@ class TestWorkerRuns(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertEqual(response.json(), {'detail': 'You do not have an admin access to the process project.'}) + self.assertEqual(response.json(), {'detail': 'You do not have an admin access to this process.'}) - def test_update_run_local(self): + def test_update_invalid_process_mode(self): """ A user cannot update a worker run on a local process """ @@ -830,7 +905,7 @@ class TestWorkerRuns(FixtureAPITestCase): run = local_process.worker_runs.create(version=self.version_1, parents=[]) self.client.force_login(self.user) - with self.assertNumQueries(4): + with self.assertNumQueries(6): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(run.id)}), data={ @@ -839,9 +914,11 @@ class TestWorkerRuns(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'__all__': ['Cannot update a WorkerRun on a local process']}) + self.assertEqual(response.json(), { + 'process_id': ['WorkerRuns can only be created or updated on Workers or Dataset processes.'], + }) - def test_update_run_invalid_id(self): + def test_update_invalid_id(self): rev_2 = self.repo.revisions.create( hash='2', message='commit message', @@ -856,25 +933,28 @@ class TestWorkerRuns(FixtureAPITestCase): version=version_2, parents=[], ) - self.client.force_login(self.user) + with self.assertNumQueries(3): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': '12341234-1234-1234-1234-123412341234'}), data={ 'parents': [str(run_2.id)], - }, format='json' + }, + format='json', ) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_update_run_inexistant_parent(self): + def test_update_nonexistent_parent(self): self.client.force_login(self.user) - with self.assertNumQueries(8): + + with self.assertNumQueries(9): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'parents': ['12341234-1234-1234-1234-123412341234'], - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @@ -884,18 +964,19 @@ class TestWorkerRuns(FixtureAPITestCase): " same Process than this WorkerRun." ]) - def test_update_run_process_id(self): + def test_update_process_id(self): """ Process field cannot be updated """ self.client.force_login(self.user) - with self.assertNumQueries(10): + with self.assertNumQueries(11): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'process_id': str(self.process_2.id), 'parents': [], - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -946,21 +1027,23 @@ class TestWorkerRuns(FixtureAPITestCase): self.run_1.refresh_from_db() self.assertEqual(self.run_1.process.id, self.process_1.id) - def test_update_run_worker_version_id(self): + def test_update_worker_version_id(self): """ Version field cannot be updated """ self.client.force_login(self.user) dla_version = WorkerVersion.objects.get(worker__slug='dla') - with self.assertNumQueries(10): + + with self.assertNumQueries(11): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'worker_version_id': str(dla_version.id), - 'parents': [], - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), { 'id': str(self.run_1.id), 'configuration': None, @@ -1008,21 +1091,23 @@ class TestWorkerRuns(FixtureAPITestCase): self.run_1.refresh_from_db() self.assertNotEqual(self.run_1.version_id, dla_version.id) - def test_update_run_configuration(self): + def test_update_configuration(self): self.client.force_login(self.user) self.assertEqual(self.run_1.configuration, None) # Check generated summary, before updating, it should not be that verbose self.assertEqual(self.run_1.summary, f"Worker {self.run_1.version.worker.name} @ {str(self.run_1.version.id)[:6]}") - with self.assertNumQueries(11): + + with self.assertNumQueries(12): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': self.run_1.id}), data={ 'parents': [], 'configuration_id': str(self.configuration_1.id) }, - format='json' + format='json', ) self.assertEqual(response.status_code, status.HTTP_200_OK) + self.run_1.refresh_from_db() self.assertEqual(response.json(), { @@ -1078,21 +1163,21 @@ class TestWorkerRuns(FixtureAPITestCase): # Check generated summary, after the update, the configuration should be displayed as well self.assertEqual(self.run_1.summary, f"Worker {self.run_1.version.worker.name} @ {str(self.run_1.version.id)[:6]} using configuration '{self.configuration_1.name}'") - def test_update_run_invalid_configuration(self): + def test_update_invalid_configuration(self): self.client.force_login(self.user) self.assertEqual(self.run_1.configuration, None) - with self.assertNumQueries(8): + with self.assertNumQueries(9): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': self.run_1.id}), data={'parents': [], 'configuration_id': str(self.configuration_2.id)}, - format='json' + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertDictEqual(response.json(), {'configuration_id': ['The configuration must be part of the same worker.']}) - def test_update_run_process_already_started(self): + def test_update_process_already_started(self): """ Update dependencies of a worker run is not possible once the process is started """ @@ -1100,20 +1185,21 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertTrue(self.process_1.tasks.exists()) self.client.force_login(self.user) - with self.assertNumQueries(7): + with self.assertNumQueries(8): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'parents': [], - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), { - '__all__': ['Cannot update a WorkerRun on a Process that has already started'] + 'process_id': ['WorkerRuns cannot be added or updated on processes that have already started.'], }) - def test_update_run_model_version_not_allowed(self): + def test_update_model_version_not_allowed(self): """ The model_version UUID is not allowed when the related version doesn't allow model_usage """ @@ -1134,21 +1220,22 @@ class TestWorkerRuns(FixtureAPITestCase): parents=[], ) - with self.assertNumQueries(8): + with self.assertNumQueries(11): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(run_2.id)}), data={ 'model_version_id': str(self.model_version_1.id), 'parents': [] - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), { - 'model_version_id': ['This worker version does not support model usage.'] + 'model_version_id': ['This worker version does not support models.'] }) - def test_update_run_unknown_model_version(self): + def test_update_unknown_model_version(self): """ Cannot use a model version id that doesn't exist """ @@ -1176,7 +1263,8 @@ class TestWorkerRuns(FixtureAPITestCase): data={ 'model_version_id': random_model_version_uuid, 'parents': [] - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @@ -1184,7 +1272,7 @@ class TestWorkerRuns(FixtureAPITestCase): 'model_version_id': [f'Invalid pk "{random_model_version_uuid}" - object does not exist.'] }) - def test_update_run_model_version_no_access(self): + def test_update_model_version_no_access(self): """ Cannot update a worker run with a model_version UUID, when you don't have access to the model version """ @@ -1207,23 +1295,30 @@ class TestWorkerRuns(FixtureAPITestCase): # Create a model version, the user has no access to model_no_access = Model.objects.create(name='Secret model') - model_version_no_access = ModelVersion.objects.create(model=model_no_access, state=ModelVersionState.Available, size=8, hash='aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', archive_hash='bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb') + model_version_no_access = ModelVersion.objects.create( + model=model_no_access, + state=ModelVersionState.Available, + size=8, + hash='aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + archive_hash='bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', + ) - with self.assertNumQueries(10): + with self.assertNumQueries(11): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(run_2.id)}), data={ 'model_version_id': str(model_version_no_access.id), 'parents': [] - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), { - 'model_version_id': ['You do not have access to this model version.'] + 'model_version_id': ['You do not have contributor access to this model.'] }) - def test_update_run_model_version_unavailable(self): + def test_update_model_version_unavailable(self): self.client.force_login(self.user) rev_2 = self.repo.revisions.create( hash='2', @@ -1243,21 +1338,22 @@ class TestWorkerRuns(FixtureAPITestCase): self.model_version_1.state = ModelVersionState.Error self.model_version_1.save() - with self.assertNumQueries(8): + with self.assertNumQueries(11): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(run.id)}), data={ 'model_version_id': str(self.model_version_1.id), 'parents': [] - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), { - 'model_version_id': ['This model version is not in an available state.'] + 'model_version_id': ['This ModelVersion is not in an Available state.'] }) - def test_update_run_model_version_id(self): + def test_update_model_version_id(self): """ Update the worker run by adding a model_version with a worker version that supports it """ @@ -1280,15 +1376,18 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertEqual(run.model_version, None) # Check generated summary, before updating, there should be only information about the worker version self.assertEqual(run.summary, f"Worker {self.worker_1.name} @ {str(version_with_model.id)[:6]}") - with self.assertNumQueries(13): + + with self.assertNumQueries(14): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(run.id)}), data={ 'model_version_id': str(self.model_version_1.id), 'parents': [] - }, format='json' + }, + 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(), { @@ -1349,7 +1448,7 @@ class TestWorkerRuns(FixtureAPITestCase): # Check generated summary, after updating, there should be information about the model loaded self.assertEqual(run.summary, f"Worker {self.worker_1.name} @ {str(version_with_model.id)[:6]} with model {self.model_version_1.model.name} @ {str(self.model_version_1.id)[:6]}") - def test_update_run_configuration_and_model_version(self): + def test_update_configuration_and_model_version(self): """ Update the worker run by adding both a model_version and a worker configuration """ @@ -1373,16 +1472,19 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertIsNone(run.configuration) # Check generated summary, before updating, there should be only information about the worker version self.assertEqual(run.summary, f"Worker {self.worker_1.name} @ {str(version_with_model.id)[:6]}") - with self.assertNumQueries(14): + + with self.assertNumQueries(15): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(run.id)}), data={ 'model_version_id': str(self.model_version_1.id), 'configuration_id': str(self.configuration_1.id), 'parents': [] - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_200_OK) + run.refresh_from_db() self.assertEqual(response.json(), { 'id': str(run.id), @@ -1447,7 +1549,7 @@ class TestWorkerRuns(FixtureAPITestCase): # Check generated summary, after updating, there should be information about the model loaded self.assertEqual(run.summary, f"Worker {self.worker_1.name} @ {str(version_with_model.id)[:6]} with model {self.model_version_1.model.name} @ {str(self.model_version_1.id)[:6]} using configuration '{self.configuration_1.name}'") - def test_update_run(self): + def test_update(self): rev_2 = self.repo.revisions.create( hash='2', message='commit message', @@ -1463,14 +1565,17 @@ class TestWorkerRuns(FixtureAPITestCase): parents=[], ) self.client.force_login(self.user) - with self.assertNumQueries(12): + + with self.assertNumQueries(13): response = self.client.put( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'parents': [str(run_2.id)], - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_200_OK) + self.run_1.refresh_from_db() self.assertDictEqual(response.json(), { 'id': str(self.run_1.id), @@ -1517,7 +1622,52 @@ class TestWorkerRuns(FixtureAPITestCase): }, }) - def test_partial_update_run_requires_login(self): + def test_update_unique(self): + self.client.force_login(self.user) + self.version_1.model_usage = True + self.version_1.save() + cases = [ + (None, None), + (None, self.configuration_1), + (self.model_version_1, None), + (self.model_version_1, self.configuration_1), + ] + for model_version, configuration in cases: + with self.subTest(model_version=model_version, configuration=configuration): + # Erase any previous failures + self.process_1.worker_runs.exclude(id=self.run_1.id).delete() + + self.run_1.model_version = model_version + self.run_1.configuration = configuration + self.run_1.save() + + # Ensure the other run has different values before updating to avoid conflicts + run_2 = self.process_1.worker_runs.create( + version=self.version_1, + model_version=None if model_version else self.model_version_1, + configuration=None if configuration else self.configuration_1, + ) + + # Having a model version set adds three queries, having a configuration adds one + query_count = 8 + bool(model_version) * 3 + bool(configuration) + + with self.assertNumQueries(query_count): + response = self.client.put( + reverse('api:worker-run-details', kwargs={'pk': str(run_2.id)}), + data={ + # Update the second worker run to the first worker run's values to cause a conflict + 'model_version_id': str(model_version.id) if model_version else None, + 'configuration_id': str(configuration.id) if configuration else None, + }, + format='json', + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + self.assertEqual(response.json(), { + '__all__': ['A WorkerRun already exists on this process with the selected worker version, model version and configuration.'], + }) + + def test_partial_update_requires_login(self): rev_2 = self.repo.revisions.create( hash='2', message='commit message', @@ -1538,11 +1688,12 @@ class TestWorkerRuns(FixtureAPITestCase): reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'parents': [str(run_2.id)], - }, format='json' + }, + format='json', ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_partial_update_run_no_project_admin_right(self): + def test_partial_update_no_project_admin_right(self): """ A user cannot update a worker run if he has no admin access on its process project """ @@ -1555,9 +1706,9 @@ class TestWorkerRuns(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - self.assertEqual(response.json(), {'detail': 'You do not have an admin access to the process project.'}) + self.assertEqual(response.json(), {'detail': 'You do not have an admin access to this process.'}) - def test_partial_update_run_invalid_id(self): + def test_partial_update_invalid_id(self): rev_2 = self.repo.revisions.create( hash='2', message='commit message', @@ -1579,11 +1730,12 @@ class TestWorkerRuns(FixtureAPITestCase): reverse('api:worker-run-details', kwargs={'pk': '12341234-1234-1234-1234-123412341234'}), data={ 'parents': [str(run_2.id)], - }, format='json' + }, + format='json', ) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_partial_update_run_local(self): + def test_partial_update_local(self): """ A user cannot update a worker run on a local process """ @@ -1591,7 +1743,7 @@ class TestWorkerRuns(FixtureAPITestCase): run = local_process.worker_runs.create(version=self.version_1, parents=[]) self.client.force_login(self.user) - with self.assertNumQueries(4): + with self.assertNumQueries(6): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(run.id)}), data={ @@ -1600,38 +1752,44 @@ class TestWorkerRuns(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'__all__': ['Cannot update a WorkerRun on a local process']}) + self.assertEqual(response.json(), { + 'process_id': ['WorkerRuns can only be created or updated on Workers or Dataset processes.'], + }) - def test_partial_update_run_inexistant_parent(self): + def test_partial_update_nonexistent_parent(self): self.client.force_login(self.user) - with self.assertNumQueries(8): + with self.assertNumQueries(9): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'parents': ['12341234-1234-1234-1234-123412341234'], - }, format='json' + }, + format='json', ) 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" " same Process than this WorkerRun." ]) - def test_partial_update_run_process_id(self): + def test_partial_update_process_id(self): """ Process field cannot be updated """ self.client.force_login(self.user) - with self.assertNumQueries(10): + + with self.assertNumQueries(11): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'process_id': str(self.process_2.id), - 'parents': [], - }, format='json' + }, + format='json', ) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), { 'id': str(self.run_1.id), 'configuration': None, @@ -1679,21 +1837,23 @@ class TestWorkerRuns(FixtureAPITestCase): self.run_1.refresh_from_db() self.assertEqual(self.run_1.process.id, self.process_1.id) - def test_partial_update_run_worker_version_id(self): + def test_partial_update_worker_version_id(self): """ Version field cannot be updated """ self.client.force_login(self.user) dla_version = WorkerVersion.objects.get(worker__slug='dla') - with self.assertNumQueries(10): + + with self.assertNumQueries(11): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'worker_version_id': str(dla_version.id), - 'parents': [], - }, format='json' + }, + format='json', ) - self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.json(), { 'id': str(self.run_1.id), 'configuration': None, @@ -1741,19 +1901,21 @@ class TestWorkerRuns(FixtureAPITestCase): self.run_1.refresh_from_db() self.assertNotEqual(self.run_1.version_id, dla_version.id) - def test_partial_update_run_configuration(self): + def test_partial_update_configuration(self): self.client.force_login(self.user) self.assertEqual(self.run_1.configuration, None) self.assertEqual(self.run_1.summary, f"Worker {self.run_1.version.worker.name} @ {str(self.run_1.version.id)[:6]}") - with self.assertNumQueries(11): + + with self.assertNumQueries(12): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': self.run_1.id}), data={ 'configuration_id': str(self.configuration_1.id) }, - format='json' + format='json', ) self.assertEqual(response.status_code, status.HTTP_200_OK) + self.run_1.refresh_from_db() self.assertEqual(response.json(), { 'id': str(self.run_1.id), @@ -1807,38 +1969,43 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertEqual(self.run_1.configuration.id, self.configuration_1.id) self.assertEqual(self.run_1.summary, f"Worker {self.run_1.version.worker.name} @ {str(self.run_1.version.id)[:6]} using configuration '{self.configuration_1.name}'") - def test_partial_update_run_invalid_configuration(self): + def test_partial_update_invalid_configuration(self): self.client.force_login(self.user) self.assertEqual(self.run_1.configuration, None) - with self.assertNumQueries(8): + + with self.assertNumQueries(9): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': self.run_1.id}), data={'configuration_id': str(self.configuration_2.id)}, - format='json' + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertDictEqual(response.json(), {'configuration_id': ['The configuration must be part of the same worker.']}) - def test_partial_update_run_process_already_started(self): + def test_partial_update_process_already_started(self): """ Update dependencies of a worker run is not possible once the process is started """ self.process_1.start() self.assertTrue(self.process_1.tasks.exists()) self.client.force_login(self.user) - with self.assertNumQueries(7): + + with self.assertNumQueries(8): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'parents': [], - }, format='json' + }, + 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(), { - '__all__': ['Cannot update a WorkerRun on a Process that has already started'] + 'process_id': ['WorkerRuns cannot be added or updated on processes that have already started.'], }) - def test_partial_update_run_model_version_not_allowed(self): + def test_partial_update_model_version_not_allowed(self): """ The model_version UUID is not allowed when the related version doesn't allow model_usage """ @@ -1858,20 +2025,22 @@ class TestWorkerRuns(FixtureAPITestCase): version=version_no_model, parents=[], ) - with self.assertNumQueries(8): + + with self.assertNumQueries(11): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(run_2.id)}), data={ 'model_version_id': str(self.model_version_1.id), - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), { - 'model_version_id': ['This worker version does not support model usage.'] + 'model_version_id': ['This worker version does not support models.'] }) - def test_partial_update_run_unknown_model_version(self): + def test_partial_update_unknown_model_version(self): """ Cannot use a model version id that doesn't exist """ @@ -1898,7 +2067,8 @@ class TestWorkerRuns(FixtureAPITestCase): reverse('api:worker-run-details', kwargs={'pk': str(run_2.id)}), data={ 'model_version_id': random_model_version_uuid, - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @@ -1906,7 +2076,7 @@ class TestWorkerRuns(FixtureAPITestCase): 'model_version_id': [f'Invalid pk "{random_model_version_uuid}" - object does not exist.'] }) - def test_partial_update_run_model_version_no_access(self): + def test_partial_update_model_version_no_access(self): """ Cannot update a worker run with a model_version UUID, when you don't have access to the model version """ @@ -1931,20 +2101,21 @@ class TestWorkerRuns(FixtureAPITestCase): model_no_access = Model.objects.create(name='Secret model') model_version_no_access = ModelVersion.objects.create(model=model_no_access, state=ModelVersionState.Available, size=8, hash='aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', archive_hash='bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb') - with self.assertNumQueries(10): + with self.assertNumQueries(11): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(run_2.id)}), data={ 'model_version_id': str(model_version_no_access.id), - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), { - 'model_version_id': ['You do not have access to this model version.'] + 'model_version_id': ['You do not have contributor access to this model.'], }) - def test_partial_update_run_model_version_unavailable(self): + def test_partial_update_model_version_unavailable(self): self.client.force_login(self.user) rev_2 = self.repo.revisions.create( hash='2', @@ -1964,21 +2135,22 @@ class TestWorkerRuns(FixtureAPITestCase): self.model_version_1.state = ModelVersionState.Error self.model_version_1.save() - with self.assertNumQueries(8): + with self.assertNumQueries(11): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(run.id)}), data={ 'model_version_id': str(self.model_version_1.id), 'parents': [] - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.json(), { - 'model_version_id': ['This model version is not in an available state.'] + 'model_version_id': ['This ModelVersion is not in an Available state.'], }) - def test_partial_update_run_model_version(self): + def test_partial_update_model_version(self): """ Update the worker run by adding a model_version with a worker version that supports it """ @@ -2000,12 +2172,14 @@ class TestWorkerRuns(FixtureAPITestCase): ) self.assertIsNone(run.model_version_id) self.assertEqual(run.summary, f"Worker {version_with_model.worker.name} @ {str(version_with_model.id)[:6]}") - with self.assertNumQueries(13): + + with self.assertNumQueries(14): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(run.id)}), data={ 'model_version_id': str(self.model_version_1.id), - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -2067,7 +2241,7 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertEqual(run.model_version_id, self.model_version_1.id) self.assertEqual(run.summary, f"Worker {version_with_model.worker.name} @ {str(version_with_model.id)[:6]} with model {self.model_version_1.model.name} @ {str(self.model_version_1.id)[:6]}") - def test_partial_update_run_model_version_with_configuration(self): + def test_partial_update_model_version_with_configuration(self): """ Updating the worker run by adding a model_version with a worker version doesn't erase previously loaded worker configuration @@ -2090,14 +2264,17 @@ class TestWorkerRuns(FixtureAPITestCase): configuration=self.configuration_1 ) self.assertEqual(run.model_version_id, None) - with self.assertNumQueries(13): + + with self.assertNumQueries(14): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(run.id)}), data={ 'model_version_id': str(self.model_version_1.id), - }, format='json' + }, + format='json', ) self.assertEqual(response.status_code, status.HTTP_200_OK) + run.refresh_from_db() self.assertEqual(response.json(), { 'id': str(run.id), @@ -2161,7 +2338,7 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertEqual(run.model_version_id, self.model_version_1.id) self.assertEqual(run.configuration_id, self.configuration_1.id) - def test_partial_update_run(self): + def test_partial_update(self): rev_2 = self.repo.revisions.create( hash='2', message='commit message', @@ -2177,14 +2354,17 @@ class TestWorkerRuns(FixtureAPITestCase): parents=[], ) self.client.force_login(self.user) - with self.assertNumQueries(12): + + with self.assertNumQueries(13): response = self.client.patch( reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}), data={ 'parents': [str(run_2.id)], - }, format='json' + }, + 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(), { 'id': str(self.run_1.id), @@ -2231,33 +2411,83 @@ class TestWorkerRuns(FixtureAPITestCase): }, }) - def test_delete_run_requires_login(self): - response = self.client.delete( - reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) - ) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + def test_partial_update_unique(self): + self.client.force_login(self.user) + self.version_1.model_usage = True + self.version_1.save() + cases = [ + (None, None), + (None, self.configuration_1), + (self.model_version_1, None), + (self.model_version_1, self.configuration_1), + ] + for model_version, configuration in cases: + with self.subTest(model_version=model_version, configuration=configuration): + # Erase any previous failures + self.process_1.worker_runs.exclude(id=self.run_1.id).delete() + + self.run_1.model_version = model_version + self.run_1.configuration = configuration + self.run_1.save() + + # Ensure the other run has different values before updating to avoid conflicts + run_2 = self.process_1.worker_runs.create( + version=self.version_1, + model_version=None if model_version else self.model_version_1, + configuration=None if configuration else self.configuration_1, + ) + + # Having a model version set adds three queries, having a configuration adds one + query_count = 8 + bool(model_version) * 3 + bool(configuration) + + with self.assertNumQueries(query_count): + response = self.client.patch( + reverse('api:worker-run-details', kwargs={'pk': str(run_2.id)}), + data={ + # Update the second worker run to the first worker run's values to cause a conflict + 'model_version_id': str(model_version.id) if model_version else None, + 'configuration_id': str(configuration.id) if configuration else None, + }, + format='json', + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - def test_delete_run_invalid_id(self): + self.assertEqual(response.json(), { + '__all__': ['A WorkerRun already exists on this process with the selected worker version, model version and configuration.'], + }) + + def test_delete_requires_login(self): + with self.assertNumQueries(0): + response = self.client.delete( + reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) + ) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_delete_invalid_id(self): self.client.force_login(self.user) - response = self.client.delete( - reverse('api:worker-run-details', kwargs={'pk': '12341234-1234-1234-1234-123412341234'}) - ) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + with self.assertNumQueries(3): + response = self.client.delete( + reverse('api:worker-run-details', kwargs={'pk': '12341234-1234-1234-1234-123412341234'}) + ) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_delete_run_no_worker_execution_right(self): + def test_delete_no_worker_execution_right(self): """ A user can delete a worker run with no right on its worker but an admin access to the process project """ self.worker_1.memberships.update(level=Role.Guest.value) self.client.force_login(self.user) - response = self.client.delete( - reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) - ) - self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + + with self.assertNumQueries(8): + response = self.client.delete( + reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) + ) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + with self.assertRaises(WorkerRun.DoesNotExist): self.run_1.refresh_from_db() - def test_delete_run_local(self): + def test_delete_local(self): """ A user cannot delete a worker run on a local process """ @@ -2265,24 +2495,27 @@ class TestWorkerRuns(FixtureAPITestCase): run = local_process.worker_runs.create(version=self.version_1, parents=[]) self.client.force_login(self.user) - with self.assertNumQueries(3): + with self.assertNumQueries(4): response = self.client.delete( reverse('api:worker-run-details', kwargs={'pk': str(run.id)}), ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'__all__': ['Cannot update a WorkerRun on a local process']}) + self.assertEqual(response.json(), ['WorkerRuns can only be deleted from Workers or Dataset processes.']) - def test_delete_run(self): + def test_delete(self): self.client.force_login(self.user) - response = self.client.delete( - reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) - ) - self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + + with self.assertNumQueries(8): + response = self.client.delete( + reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) + ) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + with self.assertRaises(WorkerRun.DoesNotExist): self.run_1.refresh_from_db() - def test_delete_run_with_children(self): + def test_delete_with_children(self): rev_2 = self.repo.revisions.create( hash='2', message='commit message', @@ -2315,10 +2548,12 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertTrue(self.run_1.id in run_2.parents) self.assertTrue(self.run_1.id in run_3.parents) self.client.force_login(self.user) - response = self.client.delete( - reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) - ) - self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + + with self.assertNumQueries(8): + response = self.client.delete( + reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) + ) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) with self.assertRaises(WorkerRun.DoesNotExist): self.run_1.refresh_from_db() @@ -2327,18 +2562,21 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertEqual(run_2.parents, []) self.assertEqual(run_3.parents, [run_2.id]) - def test_delete_run_started_process(self): + def test_delete_started_process(self): """ A user shouldn't be able to delete the worker run of a started process """ self.client.force_login(self.user) self.process_1.start() self.process_1.tasks.update(state=State.Running) - response = self.client.delete( - reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) - ) - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual(response.json(), {'__all__': ["Cannot update a WorkerRun on a Process that has already started"]}) + + with self.assertNumQueries(6): + response = self.client.delete( + reverse('api:worker-run-details', kwargs={'pk': str(self.run_1.id)}) + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + self.assertEqual(response.json(), ['WorkerRuns cannot be deleted from a process that has already started.']) def test_build_task_no_parent(self): self.version_1.docker_image_id = self.artifact.id @@ -2402,6 +2640,7 @@ class TestWorkerRuns(FixtureAPITestCase): self.assertEqual(task.command, None) self.assertEqual(task.image_artifact, self.artifact) self.assertEqual(task.shm_size, None) + self.assertEqual(parent_slugs, [f'reco_{str(self.version_1.id)[0:6]}']) self.assertEqual(task.env, { 'ARKINDEX_PROCESS_ID': '12345', 'ARKINDEX_TASK_TOKEN': str(task.token),