Skip to content

Commit 11f24e0

Browse files
committed
fix: Long DELETE project call
1 parent 81a5e59 commit 11f24e0

File tree

5 files changed

+64
-4
lines changed

5 files changed

+64
-4
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Generated by Django 3.2.23 on 2024-01-31 18:47
2+
3+
from django.db import migrations, models
4+
import django.db.models.deletion
5+
6+
7+
class Migration(migrations.Migration):
8+
9+
dependencies = [
10+
('projects', '0021_add_identity_overrides_migration_status'),
11+
('environments', '0033_add_environment_feature_state_version_logic'),
12+
]
13+
14+
operations = [
15+
migrations.AlterField(
16+
model_name='environment',
17+
name='project',
18+
field=models.ForeignKey(help_text='Changing the project selected will remove all previous Feature States for the previously associated projects Features that are related to this Environment. New default Feature States will be created for the new selected projects Features for this Environment.', on_delete=django.db.models.deletion.DO_NOTHING, related_name='environments', to='projects.project'),
19+
),
20+
]

api/environments/models.py

+8-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,14 @@ class Environment(
8686
" Environment. New default Feature States will be created for the new"
8787
" selected projects Features for this Environment."
8888
),
89-
on_delete=models.CASCADE,
89+
# Deleting large environments take significant amount of queries due to
90+
# audit log signals and lifecycle events for related objects.
91+
# For now, decouple environment deletion from project deletion
92+
# to be able to perform it asynchronously.
93+
#
94+
# We choose `DO_NOTHING` here as we're only taking project soft deletes
95+
# into account
96+
on_delete=models.DO_NOTHING,
9097
)
9198

9299
api_key = models.CharField(

api/environments/tasks.py

+5
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,8 @@ def delete_environment_from_dynamo(api_key: str, environment_id: str):
5151

5252
# Delete environment_v2 documents
5353
environment_v2_wrapper.delete_environment(environment_id)
54+
55+
56+
@register_task_handler()
57+
def delete_environment(environment_id: int) -> None:
58+
Environment.objects.get(id=environment_id).delete()

api/projects/views.py

+7-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
# -*- coding: utf-8 -*-
2-
from __future__ import unicode_literals
3-
41
from django.conf import settings
52
from django.utils.decorators import method_decorator
63
from drf_yasg import openapi
@@ -16,6 +13,7 @@
1613
from environments.dynamodb.migrator import IdentityMigrator
1714
from environments.identities.models import Identity
1815
from environments.serializers import EnvironmentSerializerLight
16+
from environments.tasks import delete_environment
1917
from permissions.permissions_calculator import get_project_permission_data
2018
from permissions.serializers import (
2119
PermissionModelSerializer,
@@ -178,6 +176,12 @@ def migrate_to_edge(self, request: Request, pk: int = None):
178176
identity_migrator.trigger_migration()
179177
return Response(status=status.HTTP_202_ACCEPTED)
180178

179+
def perform_destroy(self, instance: Project) -> None:
180+
instance.delete()
181+
182+
for environment_id in instance.environments.values_list("id", flat=True):
183+
delete_environment.delay(kwargs={"environment_id": environment_id})
184+
181185

182186
class BaseProjectPermissionsViewSet(viewsets.ModelViewSet):
183187
model_class = None

api/tests/unit/projects/test_unit_projects_views.py

+24
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
from django.urls import reverse
77
from django.utils import timezone
88
from pytest_lazyfixture import lazy_fixture
9+
from pytest_mock import MockerFixture
910
from rest_framework import status
1011
from rest_framework.test import APIClient
1112

1213
from environments.dynamodb.types import ProjectIdentityMigrationStatus
1314
from environments.identities.models import Identity
15+
from environments.models import Environment
1416
from features.models import Feature, FeatureSegment
1517
from organisations.models import Organisation, OrganisationRole
1618
from organisations.permissions.models import (
@@ -783,3 +785,25 @@ def test_get_project_data_by_id(
783785
assert response_json["total_features"] == num_features
784786
assert response_json["total_segments"] == num_segments
785787
assert response_json["show_edge_identity_overrides_for_feature"] is False
788+
789+
790+
def test_delete_project__calls_expected(
791+
admin_client: APIClient,
792+
project: Project,
793+
environment: Environment,
794+
mocker: MockerFixture,
795+
) -> None:
796+
# Given
797+
url = reverse("api-v1:projects:project-detail", args=[project.id])
798+
799+
delete_environment_mock = mocker.patch("projects.views.delete_environment")
800+
801+
# When
802+
admin_client.delete(url)
803+
804+
# Then
805+
project.refresh_from_db()
806+
assert project.deleted
807+
delete_environment_mock.delay.assert_called_once_with(
808+
kwargs={"environment_id": environment.id}
809+
)

0 commit comments

Comments
 (0)