Skip to content

Commit 8a2a39a

Browse files
daniel-slaughdaniel-slaugh
committed
Make deleting orchestration systems safe
1 parent 3c2ea75 commit 8a2a39a

File tree

3 files changed

+118
-15
lines changed

3 files changed

+118
-15
lines changed

‎domains/etl/services/orchestration_system.py‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from django.http import HttpResponse
55
from django.contrib.auth import get_user_model
66
from django.db import IntegrityError
7-
from django.db.models import QuerySet
7+
from django.db.models import Count, QuerySet
88
from domains.iam.models import APIKey
99
from domains.etl.models import OrchestrationSystem
1010
from interfaces.api.schemas import (
@@ -23,6 +23,9 @@
2323

2424

2525
class OrchestrationSystemService(ServiceUtils):
26+
@staticmethod
27+
def annotate_task_count(queryset: QuerySet) -> QuerySet:
28+
return queryset.annotate(task_count=Count("tasks", distinct=True))
2629

2730
def get_orchestration_system_for_action(
2831
self,
@@ -33,7 +36,9 @@ def get_orchestration_system_for_action(
3336
raise_400: bool = False,
3437
):
3538
try:
36-
orchestration_system = OrchestrationSystem.objects
39+
orchestration_system = self.annotate_task_count(
40+
OrchestrationSystem.objects
41+
)
3742
if expand_related:
3843
orchestration_system = self.select_expanded_fields(orchestration_system)
3944
orchestration_system = orchestration_system.get(pk=uid)
@@ -73,7 +78,7 @@ def list(
7378
filtering: Optional[dict] = None,
7479
expand_related: Optional[bool] = None,
7580
):
76-
queryset = OrchestrationSystem.objects
81+
queryset = self.annotate_task_count(OrchestrationSystem.objects)
7782

7883
for field in [
7984
"workspace_id",
@@ -198,6 +203,15 @@ def delete(self, principal: User | APIKey, uid: uuid.UUID):
198203
principal=principal, uid=uid, action="delete", expand_related=True
199204
)
200205

206+
linked_task_count = orchestration_system.tasks.count()
207+
if linked_task_count > 0:
208+
task_label = "task" if linked_task_count == 1 else "tasks"
209+
verb = "is" if linked_task_count == 1 else "are"
210+
raise HttpError(
211+
409,
212+
f"Cannot delete orchestration system while {linked_task_count} {task_label} {verb} still linked to it.",
213+
)
214+
201215
orchestration_system.delete()
202216

203217
return "Orchestration system deleted"

‎interfaces/api/schemas/orchestration_system.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,13 @@ class OrchestrationSystemQueryParameters(CollectionQueryParameters):
4141
class OrchestrationSystemSummaryResponse(BaseGetResponse, OrchestrationSystemFields):
4242
id: uuid.UUID
4343
workspace_id: Optional[uuid.UUID] = None
44+
task_count: int = 0
4445

4546

4647
class OrchestrationSystemDetailResponse(BaseGetResponse, OrchestrationSystemFields):
4748
id: uuid.UUID
4849
workspace: Optional[WorkspaceSummaryResponse] = None
50+
task_count: int = 0
4951

5052

5153
class OrchestrationSystemPostBody(BasePostBody, OrchestrationSystemFields):

‎tests/etl/services/test_orchestration_system.py‎

Lines changed: 99 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,14 @@ def test_list_orchestration_system(
8282
assert Counter(
8383
str(orchestration_system.name) for orchestration_system in result
8484
) == Counter(orchestration_system_names)
85+
task_counts = {
86+
str(orchestration_system.name): orchestration_system.task_count
87+
for orchestration_system in result
88+
}
89+
if "HydroServer" in orchestration_system_names:
90+
assert task_counts["HydroServer"] == 0
91+
if "Test Streaming Data Loader" in orchestration_system_names:
92+
assert task_counts["Test Streaming Data Loader"] == 1
8593
assert (
8694
OrchestrationSystemSummaryResponse.from_orm(orchestration_system)
8795
for orchestration_system in result
@@ -162,6 +170,7 @@ def test_get_orchestration_system(
162170
principal=get_principal(principal), uid=uuid.UUID(orchestration_system)
163171
)
164172
assert orchestration_system_get.name == message
173+
assert orchestration_system_get.task_count == 1
165174
assert OrchestrationSystemSummaryResponse.from_orm(orchestration_system_get)
166175

167176

@@ -296,12 +305,42 @@ def test_create_orchestration_system(
296305
@pytest.mark.parametrize(
297306
"principal, orchestration_system, message, error_code",
298307
[
299-
("admin", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
300-
("admin", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
301-
("owner", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
302-
("owner", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
303-
("editor", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
304-
("editor", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
308+
(
309+
"admin",
310+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
311+
"Cannot delete orchestration system while 1 task is still linked to it.",
312+
409,
313+
),
314+
(
315+
"admin",
316+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
317+
"Cannot delete orchestration system while 1 task is still linked to it.",
318+
409,
319+
),
320+
(
321+
"owner",
322+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
323+
"Cannot delete orchestration system while 1 task is still linked to it.",
324+
409,
325+
),
326+
(
327+
"owner",
328+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
329+
"Cannot delete orchestration system while 1 task is still linked to it.",
330+
409,
331+
),
332+
(
333+
"editor",
334+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
335+
"Cannot delete orchestration system while 1 task is still linked to it.",
336+
409,
337+
),
338+
(
339+
"editor",
340+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
341+
"Cannot delete orchestration system while 1 task is still linked to it.",
342+
409,
343+
),
305344
(
306345
"viewer",
307346
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
@@ -392,12 +431,42 @@ def test_edit_orchestration_system(
392431
@pytest.mark.parametrize(
393432
"principal, orchestration_system, message, error_code",
394433
[
395-
("admin", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
396-
("admin", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
397-
("owner", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
398-
("owner", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
399-
("editor", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
400-
("editor", "7cb900d2-eb11-4a59-a05b-dd02d95af312", None, None),
434+
(
435+
"admin",
436+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
437+
"Cannot delete orchestration system while 1 task is still linked to it.",
438+
409,
439+
),
440+
(
441+
"admin",
442+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
443+
"Cannot delete orchestration system while 1 task is still linked to it.",
444+
409,
445+
),
446+
(
447+
"owner",
448+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
449+
"Cannot delete orchestration system while 1 task is still linked to it.",
450+
409,
451+
),
452+
(
453+
"owner",
454+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
455+
"Cannot delete orchestration system while 1 task is still linked to it.",
456+
409,
457+
),
458+
(
459+
"editor",
460+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
461+
"Cannot delete orchestration system while 1 task is still linked to it.",
462+
409,
463+
),
464+
(
465+
"editor",
466+
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
467+
"Cannot delete orchestration system while 1 task is still linked to it.",
468+
409,
469+
),
401470
(
402471
"viewer",
403472
"7cb900d2-eb11-4a59-a05b-dd02d95af312",
@@ -475,3 +544,21 @@ def test_delete_orchestration_system(
475544
principal=get_principal(principal), uid=uuid.UUID(orchestration_system)
476545
)
477546
assert orchestration_system_delete == "Orchestration system deleted"
547+
548+
549+
def test_delete_unlinked_orchestration_system(get_principal):
550+
orchestration_system = orchestration_system_service.create(
551+
principal=get_principal("owner"),
552+
data=OrchestrationSystemPostBody(
553+
name="Temporary",
554+
workspace_id=uuid.UUID("b27c51a0-7374-462d-8a53-d97d47176c10"),
555+
orchestration_system_type="SDL",
556+
),
557+
)
558+
559+
result = orchestration_system_service.delete(
560+
principal=get_principal("owner"),
561+
uid=orchestration_system.id,
562+
)
563+
564+
assert result == "Orchestration system deleted"

0 commit comments

Comments
 (0)