Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid activating an algorithm image if the current active one is used by an evaluation in progress #3785

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions app/grandchallenge/algorithms/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
JSONEditorWidget,
MarkdownEditorInlineWidget,
)
from grandchallenge.evaluation.models import Evaluation
from grandchallenge.evaluation.utils import SubmissionKindChoices, get
from grandchallenge.groups.forms import UserGroupForm
from grandchallenge.hanging_protocols.forms import ViewContentExampleMixin
Expand Down Expand Up @@ -796,6 +797,25 @@ def clean_algorithm_image(self):
if algorithm_image.algorithm.image_upload_in_progress:
raise ValidationError("Image updating already in progress.")

active_image = algorithm_image.algorithm.active_image

if (
Evaluation.objects.filter(
submission__algorithm_image=active_image,
)
.exclude(
status__in=[
Evaluation.SUCCESS,
Evaluation.FAILURE,
Evaluation.CANCELLED,
]
)
.exists()
):
raise ValidationError(
"Current active algorithm image cannot be deactivated. An evaluation using it is in progress."
)

return algorithm_image


Expand Down
30 changes: 29 additions & 1 deletion app/grandchallenge/components/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2064,6 +2064,34 @@ def save(self, *args, **kwargs):
if self.has_changed("image") or self.has_changed("is_in_registry"):
self.update_size_in_storage()

mark_as_desired = True

if self._meta.model_name == "algorithmimage":

from grandchallenge.algorithms.models import Algorithm
from grandchallenge.evaluation.models import Evaluation

active_image = (
Algorithm.objects.filter(pk=self.algorithm.pk)
.get()
.active_image
)

if (
Evaluation.objects.filter(
submission__algorithm_image=active_image,
)
.exclude(
status__in=[
Evaluation.SUCCESS,
Evaluation.FAILURE,
Evaluation.CANCELLED,
]
)
.exists()
):
mark_as_desired = False

super().save(*args, **kwargs)

if validate_image_now:
Expand All @@ -2073,7 +2101,7 @@ def save(self, *args, **kwargs):
"app_label": self._meta.app_label,
"model_name": self._meta.model_name,
"pk": self.pk,
"mark_as_desired": True,
"mark_as_desired": mark_as_desired,
},
immutable=True,
).apply_async
Expand Down
43 changes: 42 additions & 1 deletion app/tests/algorithms_tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from grandchallenge.core.utils.access_requests import (
AccessRequestHandlingOptions,
)
from grandchallenge.evaluation.models import Evaluation
from grandchallenge.evaluation.utils import SubmissionKindChoices
from grandchallenge.verifications.models import Verification
from tests.algorithms_tests.factories import (
Expand All @@ -43,7 +44,7 @@
from tests.algorithms_tests.utils import get_algorithm_creator
from tests.components_tests.factories import ComponentInterfaceFactory
from tests.conftest import get_interface_form_data
from tests.evaluation_tests.factories import PhaseFactory
from tests.evaluation_tests.factories import EvaluationFactory, PhaseFactory
from tests.factories import (
UserFactory,
WorkstationConfigFactory,
Expand Down Expand Up @@ -1388,3 +1389,43 @@ def test_algorithm_for_phase_form_memory():
)
assert max_validator is not None
assert max_validator.limit_value == 42


@pytest.mark.django_db
def test_image_activate_form_fail_if_active_image_is_used_in_evaluation():
alg = AlgorithmFactory()
editor = UserFactory()
alg.add_editor(editor)
i1 = AlgorithmImageFactory(
algorithm=alg,
is_manifest_valid=True,
is_desired_version=True,
is_in_registry=True,
)
i2 = AlgorithmImageFactory(
algorithm=alg,
is_manifest_valid=True,
is_desired_version=False,
is_in_registry=True,
)

evaluation = EvaluationFactory(
submission__algorithm_image=i1,
time_limit=i1.algorithm.time_limit,
)

form = ImageActivateForm(
user=editor, algorithm=alg, data={"algorithm_image": i2}
)
assert i1 not in form.fields["algorithm_image"].queryset
assert i2 in form.fields["algorithm_image"].queryset
assert not form.is_valid()

evaluation.status = Evaluation.SUCCESS
evaluation.save()

form = ImageActivateForm(
user=editor, algorithm=alg, data={"algorithm_image": i2}
)

assert form.is_valid()
67 changes: 66 additions & 1 deletion app/tests/algorithms_tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
ComponentInterfaceValue,
)
from grandchallenge.components.schemas import GPUTypeChoices
from grandchallenge.evaluation.models import Evaluation
from tests.algorithms_tests.factories import (
AlgorithmFactory,
AlgorithmImageFactory,
Expand All @@ -34,10 +35,11 @@
ComponentInterfaceFactory,
ComponentInterfaceValueFactory,
)
from tests.evaluation_tests.factories import EvaluationFactory
from tests.factories import ImageFactory, UserFactory
from tests.reader_studies_tests.factories import ReaderStudyFactory
from tests.uploads_tests.factories import create_upload_from_file
from tests.utils import get_view_for_user
from tests.utils import get_view_for_user, recurse_callbacks


@pytest.mark.django_db
Expand Down Expand Up @@ -1379,3 +1381,66 @@ def test_active_credits_with_spent_credits(self):
algorithm_image.get_remaining_non_complimentary_jobs(user=user)
== 5
)


@pytest.mark.django_db
def test_algorithm_image_activation_on_create(
algorithm_image, settings, django_capture_on_commit_callbacks
):
settings.task_eager_propagates = (True,)
settings.task_always_eager = (True,)

alg = AlgorithmFactory()
editor = UserFactory()
alg.add_editor(editor)

current_active_image = AlgorithmImageFactory(
algorithm=alg,
is_manifest_valid=True,
is_desired_version=True,
is_in_registry=True,
)

evaluation = EvaluationFactory(
submission__algorithm_image=current_active_image,
time_limit=current_active_image.algorithm.time_limit,
)

assert evaluation.status == Evaluation.PENDING

with django_capture_on_commit_callbacks() as callbacks:
new_algorithm_image = AlgorithmImageFactory(algorithm=alg, image=None)

with open(algorithm_image, "rb") as f:
new_algorithm_image.image.save(
algorithm_image, ContentFile(f.read())
)

recurse_callbacks(
callbacks=callbacks,
django_capture_on_commit_callbacks=django_capture_on_commit_callbacks,
)
new_algorithm_image.refresh_from_db()

assert new_algorithm_image.is_desired_version is False

new_algorithm_image.delete()

evaluation.status = Evaluation.SUCCESS
evaluation.save()

with django_capture_on_commit_callbacks() as callbacks:
new_algorithm_image = AlgorithmImageFactory(algorithm=alg, image=None)

with open(algorithm_image, "rb") as f:
new_algorithm_image.image.save(
algorithm_image, ContentFile(f.read())
)

recurse_callbacks(
callbacks=callbacks,
django_capture_on_commit_callbacks=django_capture_on_commit_callbacks,
)
new_algorithm_image.refresh_from_db()

assert new_algorithm_image.is_desired_version is True
Loading