-
Notifications
You must be signed in to change notification settings - Fork 429
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
fix: Long DELETE
project call
#3360
Changes from all commits
11f24e0
1fa3ac5
3192bc4
92db14f
533d972
0a78c97
3131d5b
05e3fff
f630871
c901f57
114350d
e9b35ec
4dde947
9ee955a
210337b
b4fba0b
c957e0e
493f3a4
1ae84ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# Generated by Django 3.2.23 on 2024-01-31 18:47 | ||
|
||
from django.db import migrations, models | ||
import django.db.models.deletion | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('projects', '0021_add_identity_overrides_migration_status'), | ||
('environments', '0033_add_environment_feature_state_version_logic'), | ||
] | ||
|
||
operations = [ | ||
migrations.AlterField( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sqlmigrate output:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Django migrations for this silly stuff is always a bit of an eye roll |
||
model_name='environment', | ||
name='project', | ||
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'), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# Generated by Django 3.2.23 on 2024-02-01 12:12 | ||
|
||
from django.db import migrations, models | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add the sqlmigrate output for these migrations There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
import django.db.models.deletion | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('projects', '0021_add_identity_overrides_migration_status'), | ||
('features', '0062_alter_feature_segment_unique_together'), | ||
] | ||
|
||
operations = [ | ||
migrations.AlterField( | ||
model_name='feature', | ||
name='project', | ||
field=models.ForeignKey(help_text='Changing the project selected will remove previous Feature States for the previouslyassociated projects Environments that are related to this Feature. New default Feature States will be created for the new selected projects Environments for this Feature. Also this will remove any Tags associated with a feature as Tags are Project defined', on_delete=django.db.models.deletion.DO_NOTHING, related_name='features', to='projects.project'), | ||
matthewelwell marked this conversation as resolved.
Show resolved
Hide resolved
|
||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# Generated by Django 3.2.23 on 2024-02-02 10:10 | ||
|
||
from django.db import migrations, models | ||
import django.db.models.deletion | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('projects', '0021_add_identity_overrides_migration_status'), | ||
('features', '0063_detach_feature_from_project_cascade_delete'), | ||
] | ||
|
||
operations = [ | ||
migrations.AlterField( | ||
model_name='feature', | ||
name='project', | ||
field=models.ForeignKey(help_text='Changing the project selected will remove previous Feature States for the previously associated projects Environments that are related to this Feature. New default Feature States will be created for the new selected projects Environments for this Feature. Also this will remove any Tags associated with a feature as Tags are Project defined', on_delete=django.db.models.deletion.DO_NOTHING, related_name='features', to='projects.project'), | ||
), | ||
migrations.AlterField( | ||
model_name='historicalfeature', | ||
name='project', | ||
field=models.ForeignKey(blank=True, db_constraint=False, help_text='Changing the project selected will remove previous Feature States for the previously associated projects Environments that are related to this Feature. New default Feature States will be created for the new selected projects Environments for this Feature. Also this will remove any Tags associated with a feature as Tags are Project defined', null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='projects.project'), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,3 +22,29 @@ def migrate_project_environments_to_v2(project_id: int) -> None: | |
IdentityOverridesV2MigrationStatus.COMPLETE | ||
) | ||
project.save() | ||
|
||
|
||
@register_task_handler() | ||
def handle_cascade_delete(project_id: int) -> None: | ||
""" | ||
Due to the combination of soft deletes and audit log functionality, | ||
cascade deletes for a project can take a long time for large projects. | ||
This task delegates the cascade delete to tasks for the main related | ||
entities for a project. | ||
""" | ||
|
||
from environments.tasks import delete_environment | ||
from features.tasks import delete_feature | ||
from projects.models import Project | ||
from segments.tasks import delete_segment | ||
Comment on lines
+36
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand the local import for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ugh, yeah, I tried to resolve this but I ended up just diving into a hell hole of circular dependencies. It annoyingly made testing a little harder because I can't mock the imports, but I do feel like it's not a major issue for tasks to have local imports given their place in the stack. |
||
|
||
project = Project.objects.all_with_deleted().get(id=project_id) | ||
|
||
for environment_id in project.environments.values_list("id", flat=True): | ||
delete_environment.delay(kwargs={"environment_id": environment_id}) | ||
|
||
for segment_id in project.segments.values_list("id", flat=True): | ||
delete_segment.delay(kwargs={"segment_id": segment_id}) | ||
|
||
for feature_id in project.features.values_list("id", flat=True): | ||
delete_feature.delay(kwargs={"feature_id": feature_id}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# Generated by Django 3.2.23 on 2024-02-01 13:45 | ||
|
||
from django.db import migrations, models | ||
import django.db.models.deletion | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('projects', '0021_add_identity_overrides_migration_status'), | ||
('segments', '0019_add_audit_to_condition'), | ||
] | ||
|
||
operations = [ | ||
migrations.AlterField( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sqlmigrate output:
|
||
model_name='segment', | ||
name='project', | ||
field=models.ForeignKey(on_delete=django.db.models.deletion.DO_NOTHING, related_name='segments', to='projects.project'), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
from segments.models import Segment | ||
from task_processor.decorators import register_task_handler | ||
|
||
|
||
@register_task_handler() | ||
def delete_segment(segment_id: int) -> None: | ||
Segment.objects.get(pk=segment_id).delete() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is arguably unrelated to this PR but I think it's a good improvement here to prevent unnecessary data in the AuditLog table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it's sorta unrelated but it's a good addition.