Skip to content

Commit 223c73d

Browse files
authored
chore(alerts): Remove excluded projects trigger exclusion (#81020)
A follow up to #79700 and a step towards removing the `AlertRuleExcludedProjects` and `AlertRuleTriggerExclusion` models. This sets the foreign keys' `db_constraint` to False and removes any lingering code that references the tables or usage of the columns. I removed references to the `AlertRule` `excluded_projects` and `include_all_projects` fields from the serializer as well since they're unused. The only place that still references them is in the [backup tests](https://github.com/getsentry/sentry/blob/master/src/sentry/testutils/helpers/backups.py#L520) that can't be removed until we're removing the table in the next step.
1 parent a6e09ad commit 223c73d

File tree

11 files changed

+66
-68
lines changed

11 files changed

+66
-68
lines changed

migrations_lockfile.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ remote_subscriptions: 0003_drop_remote_subscription
1515

1616
replays: 0004_index_together
1717

18-
sentry: 0792_add_unique_index_apiauthorization
18+
sentry: 0793_remove_db_constraint_alert_rule_exclusion
1919

2020
social_auth: 0002_default_auto_field
2121

src/sentry/incidents/endpoints/organization_alert_rule_details.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,6 @@ class OrganizationAlertRuleDetailsPutSerializer(serializers.Serializer):
212212
owner = ActorField(
213213
required=False, allow_null=True, help_text="The ID of the team or user that owns the rule."
214214
)
215-
excludedProjects = serializers.ListField(
216-
child=ProjectField(scope="project:read"), required=False
217-
)
218215
thresholdPeriod = serializers.IntegerField(required=False, default=1, min_value=1, max_value=20)
219216
monitorType = serializers.IntegerField(
220217
required=False,

src/sentry/incidents/endpoints/organization_alert_rule_index.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -409,9 +409,6 @@ class OrganizationAlertRuleIndexPostSerializer(serializers.Serializer):
409409
owner = ActorField(
410410
required=False, allow_null=True, help_text="The ID of the team or user that owns the rule."
411411
)
412-
excludedProjects = serializers.ListField(
413-
child=ProjectField(scope="project:read"), required=False
414-
)
415412
thresholdPeriod = serializers.IntegerField(required=False, default=1, min_value=1, max_value=20)
416413
monitorType = serializers.IntegerField(
417414
required=False,

src/sentry/incidents/endpoints/serializers/alert_rule.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
AlertRule,
1818
AlertRuleActivity,
1919
AlertRuleActivityType,
20-
AlertRuleExcludedProjects,
2120
AlertRuleTrigger,
2221
AlertRuleTriggerAction,
2322
)
@@ -40,7 +39,6 @@
4039
class AlertRuleSerializerResponseOptional(TypedDict, total=False):
4140
environment: str | None
4241
projects: list[str] | None
43-
excludedProjects: list[dict] | None
4442
queryType: int | None
4543
resolveThreshold: float | None
4644
dataset: str | None
@@ -63,8 +61,6 @@ class AlertRuleSerializerResponseOptional(TypedDict, total=False):
6361
"status",
6462
"resolution",
6563
"thresholdPeriod",
66-
"includeAllProjects",
67-
"excludedProjects",
6864
"weeklyAvg",
6965
"totalThisWeek",
7066
"latestIncident",
@@ -89,7 +85,6 @@ class AlertRuleSerializerResponse(AlertRuleSerializerResponseOptional):
8985
resolution: float
9086
thresholdPeriod: int
9187
triggers: list[dict]
92-
includeAllProjects: bool
9388
dateModified: datetime
9489
dateCreated: datetime
9590
createdBy: dict
@@ -309,7 +304,6 @@ def serialize(
309304
"thresholdPeriod": obj.threshold_period,
310305
"triggers": attrs.get("triggers", []),
311306
"projects": sorted(attrs.get("projects", [])),
312-
"includeAllProjects": obj.include_all_projects,
313307
"owner": attrs.get("owner", None),
314308
"originalAlertRuleId": attrs.get("originalAlertRuleId", None),
315309
"comparisonDelta": obj.comparison_delta / 60 if obj.comparison_delta else None,
@@ -343,13 +337,6 @@ def get_attrs(
343337
self, item_list: Sequence[Any], user: User | RpcUser, **kwargs: Any
344338
) -> defaultdict[AlertRule, Any]:
345339
result = super().get_attrs(item_list, user, **kwargs)
346-
alert_rules = {item.id: item for item in item_list}
347-
for alert_rule_id, project_slug in AlertRuleExcludedProjects.objects.filter(
348-
alert_rule__in=item_list
349-
).values_list("alert_rule_id", "project__slug"):
350-
exclusions = result[alert_rules[alert_rule_id]].setdefault("excluded_projects", [])
351-
exclusions.append(project_slug)
352-
353340
query_to_alert_rule = {ar.snuba_query_id: ar for ar in item_list}
354341

355342
for event_type in SnubaQueryEventType.objects.filter(
@@ -366,7 +353,6 @@ def serialize(
366353
self, obj: AlertRule, attrs: Mapping[Any, Any], user: User | RpcUser, **kwargs
367354
) -> AlertRuleSerializerResponse:
368355
data = super().serialize(obj, attrs, user)
369-
data["excludedProjects"] = sorted(attrs.get("excluded_projects", []))
370356
data["eventTypes"] = sorted(attrs.get("event_types", []))
371357
data["snooze"] = False
372358
return data

src/sentry/incidents/endpoints/serializers/alert_rule_trigger.py

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,7 @@
55

66
from sentry.api.serializers import Serializer, register, serialize
77
from sentry.incidents.endpoints.utils import translate_threshold
8-
from sentry.incidents.models.alert_rule import (
9-
AlertRuleTrigger,
10-
AlertRuleTriggerAction,
11-
AlertRuleTriggerExclusion,
12-
)
8+
from sentry.incidents.models.alert_rule import AlertRuleTrigger, AlertRuleTriggerAction
139

1410

1511
@register(AlertRuleTrigger)
@@ -45,20 +41,3 @@ def serialize(self, obj, attrs, user, **kwargs):
4541
"dateCreated": obj.date_added,
4642
"actions": attrs.get("actions", []),
4743
}
48-
49-
50-
class DetailedAlertRuleTriggerSerializer(AlertRuleTriggerSerializer):
51-
def get_attrs(self, item_list, user, **kwargs):
52-
triggers = {item.id: item for item in item_list}
53-
result: dict[str, dict[str, list[str]]] = defaultdict(lambda: defaultdict(list))
54-
for trigger_id, project_slug in AlertRuleTriggerExclusion.objects.filter(
55-
alert_rule_trigger__in=item_list
56-
).values_list("alert_rule_trigger_id", "query_subscription__project__slug"):
57-
if project_slug is not None:
58-
result[triggers[trigger_id]]["excludedProjects"].append(project_slug)
59-
return result
60-
61-
def serialize(self, obj, attrs, user, **kwargs):
62-
data = super().serialize(obj, attrs, user, **kwargs)
63-
data["excludedProjects"] = sorted(attrs.get("excludedProjects", []))
64-
return data

src/sentry/incidents/models/alert_rule.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ class AlertRuleExcludedProjects(Model):
251251

252252
__relocation_scope__ = RelocationScope.Organization
253253

254-
alert_rule = FlexibleForeignKey("sentry.AlertRule", db_index=False)
254+
alert_rule = FlexibleForeignKey("sentry.AlertRule", db_index=False, db_constraint=False)
255255
project = FlexibleForeignKey("sentry.Project", db_constraint=False)
256256
date_added = models.DateTimeField(default=timezone.now)
257257

@@ -490,8 +490,10 @@ class AlertRuleTriggerExclusion(Model):
490490

491491
__relocation_scope__ = RelocationScope.Organization
492492

493-
alert_rule_trigger = FlexibleForeignKey("sentry.AlertRuleTrigger", related_name="exclusions")
494-
query_subscription = FlexibleForeignKey("sentry.QuerySubscription")
493+
alert_rule_trigger = FlexibleForeignKey(
494+
"sentry.AlertRuleTrigger", related_name="exclusions", db_constraint=False
495+
)
496+
query_subscription = FlexibleForeignKey("sentry.QuerySubscription", db_constraint=False)
495497
date_added = models.DateTimeField(default=timezone.now)
496498

497499
class Meta:

src/sentry/incidents/serializers/alert_rule.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,6 @@ class AlertRuleSerializer(CamelSnakeModelSerializer[AlertRule]):
7676
required=False,
7777
max_length=1,
7878
)
79-
excluded_projects = serializers.ListField(
80-
child=ProjectField(scope="project:read"), required=False
81-
)
8279
triggers = serializers.ListField(required=True)
8380
query_type = serializers.IntegerField(required=False)
8481
dataset = serializers.CharField(required=False)
@@ -123,8 +120,6 @@ class Meta:
123120
"comparison_delta",
124121
"aggregate",
125122
"projects",
126-
"include_all_projects",
127-
"excluded_projects",
128123
"triggers",
129124
"event_types",
130125
"monitor_type",
@@ -136,7 +131,6 @@ class Meta:
136131
]
137132
extra_kwargs = {
138133
"name": {"min_length": 1, "max_length": 256},
139-
"include_all_projects": {"default": False},
140134
"threshold_type": {"required": True},
141135
"resolve_threshold": {"required": False},
142136
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
# Generated by Django 5.1.1 on 2024-11-22 17:43
2+
3+
import django.db.models.deletion
4+
from django.db import migrations
5+
6+
import sentry.db.models.fields.foreignkey
7+
from sentry.new_migrations.migrations import CheckedMigration
8+
9+
10+
class Migration(CheckedMigration):
11+
# This flag is used to mark that a migration shouldn't be automatically run in production.
12+
# This should only be used for operations where it's safe to run the migration after your
13+
# code has deployed. So this should not be used for most operations that alter the schema
14+
# of a table.
15+
# Here are some things that make sense to mark as post deployment:
16+
# - Large data migrations. Typically we want these to be run manually so that they can be
17+
# monitored and not block the deploy for a long period of time while they run.
18+
# - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
19+
# run this outside deployments so that we don't block them. Note that while adding an index
20+
# is a schema change, it's completely safe to run the operation after the code has deployed.
21+
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
22+
23+
is_post_deployment = False
24+
25+
dependencies = [
26+
("sentry", "0792_add_unique_index_apiauthorization"),
27+
]
28+
29+
operations = [
30+
migrations.AlterField(
31+
model_name="alertruleexcludedprojects",
32+
name="alert_rule",
33+
field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
34+
db_constraint=False,
35+
db_index=False,
36+
on_delete=django.db.models.deletion.CASCADE,
37+
to="sentry.alertrule",
38+
),
39+
),
40+
migrations.AlterField(
41+
model_name="alertruletriggerexclusion",
42+
name="alert_rule_trigger",
43+
field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
44+
db_constraint=False,
45+
on_delete=django.db.models.deletion.CASCADE,
46+
related_name="exclusions",
47+
to="sentry.alertruletrigger",
48+
),
49+
),
50+
migrations.AlterField(
51+
model_name="alertruletriggerexclusion",
52+
name="query_subscription",
53+
field=sentry.db.models.fields.foreignkey.FlexibleForeignKey(
54+
db_constraint=False,
55+
on_delete=django.db.models.deletion.CASCADE,
56+
to="sentry.querysubscription",
57+
),
58+
),
59+
]

tests/sentry/incidents/endpoints/serializers/test_alert_rule.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ def assert_alert_rule_serialized(
5454
assert result["resolution"] == alert_rule.snuba_query.resolution / 60
5555
assert result["thresholdPeriod"] == alert_rule.threshold_period
5656
assert result["projects"] == alert_rule_projects
57-
assert result["includeAllProjects"] == alert_rule.include_all_projects
5857
if alert_rule.created_by_id:
5958
created_by = user_service.get_user(user_id=alert_rule.created_by_id)
6059
assert created_by is not None
@@ -264,7 +263,6 @@ def test_simple(self):
264263
result = serialize(alert_rule, serializer=DetailedAlertRuleSerializer())
265264
self.assert_alert_rule_serialized(alert_rule, result)
266265
assert sorted(result["projects"]) == sorted(p.slug for p in projects)
267-
assert result["excludedProjects"] == []
268266
assert result["eventTypes"] == [SnubaQueryEventType.EventType.ERROR.name.lower()]
269267

270268
def test_triggers(self):

tests/sentry/incidents/endpoints/serializers/test_alert_rule_trigger.py

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
11
from sentry.api.serializers import serialize
2-
from sentry.incidents.endpoints.serializers.alert_rule_trigger import (
3-
DetailedAlertRuleTriggerSerializer,
4-
)
52
from sentry.incidents.logic import create_alert_rule_trigger
63
from sentry.incidents.models.alert_rule import AlertRuleDetectionType, AlertRuleThresholdType
74
from sentry.testutils.cases import TestCase
@@ -52,12 +49,3 @@ def test_comparison_below(self):
5249
trigger = create_alert_rule_trigger(alert_rule, "hi", 80)
5350
result = serialize(trigger)
5451
self.assert_alert_rule_trigger_serialized(trigger, result, 20)
55-
56-
57-
class DetailedAlertRuleTriggerSerializerTest(BaseAlertRuleTriggerSerializerTest, TestCase):
58-
def test_simple(self):
59-
alert_rule = self.create_alert_rule(resolve_threshold=200)
60-
trigger = create_alert_rule_trigger(alert_rule, "hi", 1000)
61-
result = serialize(trigger, serializer=DetailedAlertRuleTriggerSerializer())
62-
self.assert_alert_rule_trigger_serialized(trigger, result)
63-
assert result["excludedProjects"] == []

0 commit comments

Comments
 (0)