mirror/bugsink
1
0
Fork 0
mirror of https://github.com/bugsink/bugsink.git synced 2026-02-14 10:33:50 -06:00
Code Issues Packages Projects Releases Wiki Activity

Various models: .project SET_NULL => DO_NOTHING

Browse Source 
Like e45c61d6f0, but for .project.

I originally thought `SET_NULL` would be a good way to "do stuff later", but
that's only so the degree that [1] updates are cheaper than deletes and [2]
2nd-order effects (further deletes in the dep-tree) are avoided.

Now that we have explicit Project-deletion (deps-first, delayed, properly batched)
the SET_NULL behavior is always a no-op (but with cost in queries).

As a result, in the test for project deletion (which has deletes for many
of the altered models), the following 12 queries are no longer done:

```
SELECT "projects_project"."id", [..many fields..] FROM "projects_project" WHERE "projects_project"."id" = 1
DELETE FROM "projects_projectmembership" WHERE "projects_projectmembership"."project_id" IN (1)
DELETE FROM "alerts_messagingserviceconfig" WHERE "alerts_messagingserviceconfig"."project_id" IN (1)
UPDATE "releases_release" SET "project_id" = NULL WHERE "releases_release"."project_id" IN (1)
UPDATE "issues_issue" SET "project_id" = NULL WHERE "issues_issue"."project_id" IN (1)
UPDATE "issues_grouping" SET "project_id" = NULL WHERE "issues_grouping"."project_id" IN (1)
UPDATE "events_event" SET "project_id" = NULL WHERE "events_event"."project_id" IN (1)
UPDATE "tags_tagkey" SET "project_id" = NULL WHERE "tags_tagkey"."project_id" IN (1)
UPDATE "tags_tagvalue" SET "project_id" = NULL WHERE "tags_tagvalue"."project_id" IN (1)
UPDATE "tags_eventtag" SET "project_id" = NULL WHERE "tags_eventtag"."project_id" IN (1)
UPDATE "tags_issuetag" SET "project_id" = NULL WHERE "tags_issuetag"."project_id" IN (1)
```
This commit is contained in:
Klaas van Schelven
2025-07-03 21:49:49 +02:00
parent 6b9e4d8011
commit 28b2ce0eaf
15 changed files with 221 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
alerts/migrations/0002_alter_messagingserviceconfig_project.py Normal file
View File

@@ -0,0 +1,23 @@
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
# Django came up with 0014, whatever the reason, I'm sure that 0013 is at least required (as per comments there)
("projects", "0014_alter_projectmembership_project"),
("alerts", "0001_initial"),
]
operations = [
migrations.AlterField(
model_name="messagingserviceconfig",
name="project",
field=models.ForeignKey(
on_delete=django.db.models.deletion.DO_NOTHING,
related_name="service_configs",
to="projects.project",
),
),
]

2
alerts/models.py
View File

@@ -5,7 +5,7 @@ from .service_backends.slack import SlackBackend
class MessagingServiceConfig(models.Model):
project = models.ForeignKey(Project, on_delete=models.CASCADE, related_name="service_configs")
project = models.ForeignKey(Project, on_delete=models.DO_NOTHING, related_name="service_configs")
display_name = models.CharField(max_length=100, blank=False,
help_text='For display in the UI, e.g. "#general on company Slack"')

21
events/migrations/0022_alter_event_project.py Normal file
View File

@@ -0,0 +1,21 @@
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
# Django came up with 0014, whatever the reason, I'm sure that 0013 is at least required (as per comments there)
("projects", "0014_alter_projectmembership_project"),
("events", "0021_alter_do_nothing"),
]
operations = [
migrations.AlterField(
model_name="event",
name="project",
field=models.ForeignKey(
on_delete=django.db.models.deletion.DO_NOTHING, to="projects.project"
),
),
]

2
events/models.py
View File

@@ -82,7 +82,7 @@ class Event(models.Model):
# uuid4 clientside". In any case, we just rely on the envelope's event_id (required per the envelope spec).
# Not a primary key: events may be duplicated across projects
event_id = models.UUIDField(primary_key=False, null=False, editable=False, help_text="As per the sent data")
project = models.ForeignKey(Project, blank=False, null=True, on_delete=models.SET_NULL) # SET_NULL: cleanup 'later'
project = models.ForeignKey(Project, blank=False, null=False, on_delete=models.DO_NOTHING)
data = models.TextField(blank=False, null=False)

28
issues/migrations/0025_alter_grouping_project_alter_issue_project.py Normal file
View File

@@ -0,0 +1,28 @@
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
# Django came up with 0014, whatever the reason, I'm sure that 0013 is at least required (as per comments there)
("projects", "0014_alter_projectmembership_project"),
("issues", "0024_turningpoint_project_alter_not_null"),
]
operations = [
migrations.AlterField(
model_name="grouping",
name="project",
field=models.ForeignKey(
on_delete=django.db.models.deletion.DO_NOTHING, to="projects.project"
),
),
migrations.AlterField(
model_name="issue",
name="project",
field=models.ForeignKey(
on_delete=django.db.models.deletion.DO_NOTHING, to="projects.project"
),
),
]

4
issues/models.py
View File

@@ -35,7 +35,7 @@ class Issue(models.Model):
id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
project = models.ForeignKey(
"projects.Project", blank=False, null=True, on_delete=models.SET_NULL) # SET_NULL: cleanup 'later'
"projects.Project", blank=False, null=False, on_delete=models.DO_NOTHING)
is_deleted = models.BooleanField(default=False)
@@ -213,7 +213,7 @@ class Grouping(models.Model):
into a single issue. (such manual merging is not yet implemented, but the data-model is already prepared for it)
"""
project = models.ForeignKey(
"projects.Project", blank=False, null=True, on_delete=models.SET_NULL) # SET_NULL: cleanup 'later'
"projects.Project", blank=False, null=False, on_delete=models.DO_NOTHING)
grouping_key = models.TextField(blank=False, null=False)

48
projects/migrations/0013_delete_objects_pointing_to_null_project.py Normal file
View File

@@ -0,0 +1,48 @@
from django.db import migrations
def delete_objects_pointing_to_null_project(apps, schema_editor):
# Up until now, we have various models w/ .project=FK(null=True, on_delete=models.SET_NULL)
# Although it is "not expected" in the interface, project-deletion would have led to those
# objects with a null project. We're about to change that to .project=FK(null=False, ...) which
# would crash if we don't remove those objects first. Object-removal is "fine" though, because
# as per the meaning of the SET_NULL, these objects were "dangling" anyway.
# We implement this as a _single_ cross-app migration so that reasoning about the order of deletions is easy (and
# we can just copy the correct order from the project/tasks.py `preferred` variable. This cross-appness does mean
# that we must specify all dependencies here, and all the set-null migrations (from various apps) must point at this
# migration as their dependency.
# from tasks.py, but in "strings" form
preferred = [
'tags.EventTag',
'tags.IssueTag',
'tags.TagValue',
'tags.TagKey',
# 'issues.TurningPoint', # not needed, .project is already not-null (we just added it)
'events.Event',
'issues.Grouping',
# 'alerts.MessagingServiceConfig', was CASCADE (not null), so no deletion needed
# 'projects.ProjectMembership', was CASCADE (not null), so no deletion needed
'releases.Release',
'issues.Issue',
]
for model_name in preferred:
model = apps.get_model(*model_name.split('.'))
model.objects.filter(project__isnull=True).delete()
class Migration(migrations.Migration):
dependencies = [
("projects", "0012_project_is_deleted"),
("issues", "0024_turningpoint_project_alter_not_null"),
("tags", "0004_alter_do_nothing"),
("releases", "0002_release_releases_re_sort_ep_5c07c8_idx"),
("events", "0021_alter_do_nothing"),
]
operations = [
migrations.RunPython(delete_objects_pointing_to_null_project, reverse_code=migrations.RunPython.noop),
]

19
projects/migrations/0014_alter_projectmembership_project.py Normal file
View File

@@ -0,0 +1,19 @@
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
("projects", "0013_delete_objects_pointing_to_null_project"),
]
operations = [
migrations.AlterField(
model_name="projectmembership",
name="project",
field=models.ForeignKey(
on_delete=django.db.models.deletion.DO_NOTHING, to="projects.project"
),
),
]

2
projects/models.py
View File

@@ -175,7 +175,7 @@ class Project(models.Model):
class ProjectMembership(models.Model):
project = models.ForeignKey(Project, on_delete=models.CASCADE)
project = models.ForeignKey(Project, on_delete=models.DO_NOTHING)
user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)
send_email_alerts = models.BooleanField(default=None, null=True)

2
projects/tests.py
View File

@@ -66,7 +66,7 @@ class ProjectDeletionTestCase(TransactionTestCase):
# correct for bugsink/transaction.py's select_for_update for non-sqlite databases
correct_for_select_for_update = 1 if 'sqlite' not in settings.DATABASES['default']['ENGINE'] else 0
with self.assertNumQueries(40 + correct_for_select_for_update):
with self.assertNumQueries(29 + correct_for_select_for_update):
self.project.delete_deferred()
# tests run w/ TASK_ALWAYS_EAGER, so in the below we can just check the database directly

21
releases/migrations/0003_alter_release_project.py Normal file
View File

@@ -0,0 +1,21 @@
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
# Django came up with 0014, whatever the reason, I'm sure that 0013 is at least required (as per comments there)
("projects", "0014_alter_projectmembership_project"),
("releases", "0002_release_releases_re_sort_ep_5c07c8_idx"),
]
operations = [
migrations.AlterField(
model_name="release",
name="project",
field=models.ForeignKey(
on_delete=django.db.models.deletion.DO_NOTHING, to="projects.project"
),
),
]

3
releases/models.py
View File

@@ -44,8 +44,7 @@ class Release(models.Model):
# sentry does releases per-org; we don't follow that example. our belief is basically: [1] in reality releases are
# per software package and a software package is basically a bugsink project and [2] any cross-project-per-org
# analysis you might do is more likely to be in the realm of "transactions", something we don't want to support.
project = models.ForeignKey(
"projects.Project", blank=False, null=True, on_delete=models.SET_NULL) # SET_NULL: cleanup 'later'
project = models.ForeignKey("projects.Project", blank=False, null=False, on_delete=models.DO_NOTHING)
# full version as provided by either implicit (per-event) or explicit (some API) means, including package name
# max_length matches Even.release (which is deduced from Sentry)

12
releases/tests.py
View File

@@ -1,13 +1,16 @@
from django.test import TestCase as DjangoTestCase
from datetime import timedelta
from projects.models import Project
from .models import Release, ordered_releases, RE_PACKAGE_VERSION
class ReleaseTestCase(DjangoTestCase):
def test_create_and_order(self):
r0 = Release.objects.create(version="e80f98923f7426a8087009f4c629d25a35565a6a")
project = Project.objects.create(name="Test Project")
r0 = Release.objects.create(project=project, version="e80f98923f7426a8087009f4c629d25a35565a6a")
self.assertFalse(r0.is_semver)
self.assertEqual(0, r0.sort_epoch)
@@ -17,6 +20,7 @@ class ReleaseTestCase(DjangoTestCase):
# real usage too)
# * it ensures that dates are ignored when comparing r1 and r2 (r2 has a smaller date than r1, but comes later)
r1 = Release.objects.create(
project=project,
version="2a678dbbbecd2978ccaa76c326a0fb2e70073582",
date_released=r0.date_released + timedelta(seconds=10),
)
@@ -24,17 +28,17 @@ class ReleaseTestCase(DjangoTestCase):
self.assertEqual(0, r1.sort_epoch)
# switch to semver, epoch 1
r2 = Release.objects.create(version="1.0.0")
r2 = Release.objects.create(project=project, version="1.0.0")
self.assertTrue(r2.is_semver)
self.assertEqual(1, r2.sort_epoch)
# stick with semver, but use a lower version
r3 = Release.objects.create(version="0.1.0")
r3 = Release.objects.create(project=project, version="0.1.0")
self.assertTrue(r3.is_semver)
self.assertEqual(1, r3.sort_epoch)
# put in package name; this is basically ignored for ordering purposes
r4 = Release.objects.create(version="package@2.0.0")
r4 = Release.objects.create(project=project, version="package@2.0.0")
self.assertTrue(r4.is_semver)
self.assertEqual(ordered_releases(), [r0, r1, r3, r2, r4])

42
tags/migrations/0005_alter_eventtag_project_alter_issuetag_project_and_more.py Normal file
View File

@@ -0,0 +1,42 @@
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
# Django came up with 0014, whatever the reason, I'm sure that 0013 is at least required (as per comments there)
("projects", "0014_alter_projectmembership_project"),
("tags", "0004_alter_do_nothing"),
]
operations = [
migrations.AlterField(
model_name="eventtag",
name="project",
field=models.ForeignKey(
on_delete=django.db.models.deletion.DO_NOTHING, to="projects.project"
),
),
migrations.AlterField(
model_name="issuetag",
name="project",
field=models.ForeignKey(
on_delete=django.db.models.deletion.DO_NOTHING, to="projects.project"
),
),
migrations.AlterField(
model_name="tagkey",
name="project",
field=models.ForeignKey(
on_delete=django.db.models.deletion.DO_NOTHING, to="projects.project"
),
),
migrations.AlterField(
model_name="tagvalue",
name="project",
field=models.ForeignKey(
on_delete=django.db.models.deletion.DO_NOTHING, to="projects.project"
),
),
]

8
tags/models.py
View File

@@ -36,7 +36,7 @@ from tags.utils import deduce_tags, is_mostly_unique
class TagKey(models.Model):
project = models.ForeignKey(Project, blank=False, null=True, on_delete=models.SET_NULL) # SET_NULL: cleanup 'later'
project = models.ForeignKey(Project, blank=False, null=False, on_delete=models.DO_NOTHING)
key = models.CharField(max_length=32, blank=False, null=False)
# Tags that are "mostly unique" are not displayed in the issue tag counts, because the distribution of values is
@@ -54,7 +54,7 @@ class TagKey(models.Model):
class TagValue(models.Model):
project = models.ForeignKey(Project, blank=False, null=True, on_delete=models.SET_NULL) # SET_NULL: cleanup 'later'
project = models.ForeignKey(Project, blank=False, null=False, on_delete=models.DO_NOTHING)
key = models.ForeignKey(TagKey, blank=False, null=False, on_delete=models.DO_NOTHING)
value = models.CharField(max_length=200, blank=False, null=False, db_index=True)
@@ -69,7 +69,7 @@ class TagValue(models.Model):
class EventTag(models.Model):
project = models.ForeignKey(Project, blank=False, null=True, on_delete=models.SET_NULL) # SET_NULL: cleanup 'later'
project = models.ForeignKey(Project, blank=False, null=False, on_delete=models.DO_NOTHING)
# value already implies key in our current setup.
value = models.ForeignKey(TagValue, blank=False, null=False, on_delete=models.DO_NOTHING)
@@ -106,7 +106,7 @@ class EventTag(models.Model):
class IssueTag(models.Model):
project = models.ForeignKey(Project, blank=False, null=True, on_delete=models.SET_NULL) # SET_NULL: cleanup 'later'
project = models.ForeignKey(Project, blank=False, null=False, on_delete=models.DO_NOTHING)
# denormalization that allows for a single-table-index for efficient search.
key = models.ForeignKey(TagKey, blank=False, null=False, on_delete=models.DO_NOTHING)