Skip to content

Commit

Permalink
Remove post-save option
Browse files Browse the repository at this point in the history
  • Loading branch information
snigdhas committed Oct 18, 2024
1 parent 07fbda6 commit 7c93531
Show file tree
Hide file tree
Showing 10 changed files with 13 additions and 123 deletions.
10 changes: 1 addition & 9 deletions src/sentry/api/helpers/group_index/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@
import rest_framework
from django.db import IntegrityError, router, transaction
from django.db.models import Q
from django.db.models.signals import post_save
from django.utils import timezone as django_timezone
from rest_framework import serializers
from rest_framework.request import Request
from rest_framework.response import Response

from sentry import analytics, features, options
from sentry import analytics, features
from sentry.api.serializers import serialize
from sentry.api.serializers.models.actor import ActorSerializer, ActorSerializerResponse
from sentry.db.models.query import create_or_update
Expand Down Expand Up @@ -531,13 +530,6 @@ def update_groups(
group.status = GroupStatus.RESOLVED
group.substatus = None
group.resolved_at = now
if affected and not options.get("groups.enable-post-update-signal"):
post_save.send(
sender=Group,
instance=group,
created=False,
update_fields=["resolved_at", "status", "substatus"],
)
remove_group_from_inbox(
group, action=GroupInboxRemoveAction.RESOLVED, user=acting_user
)
Expand Down
8 changes: 0 additions & 8 deletions src/sentry/event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from django.core.exceptions import ValidationError
from django.db import IntegrityError, OperationalError, connection, router, transaction
from django.db.models import Func, Max
from django.db.models.signals import post_save
from django.utils.encoding import force_str
from urllib3.exceptions import MaxRetryError, TimeoutError
from usageaccountant import UsageUnit
Expand Down Expand Up @@ -1717,13 +1716,6 @@ def _handle_regression(group: Group, event: BaseEvent, release: Release | None)
transition_type="automatic",
sender="handle_regression",
)
if not options.get("groups.enable-post-update-signal"):
post_save.send(
sender=Group,
instance=group,
created=False,
update_fields=["last_seen", "active_at", "status", "substatus"],
)

follows_semver = False
resolved_in_activity = None
Expand Down
24 changes: 1 addition & 23 deletions src/sentry/issues/escalating.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from typing import Any, TypedDict

import jsonschema
from django.db.models.signals import post_save
from snuba_sdk import (
Column,
Condition,
Expand All @@ -28,7 +27,7 @@
)
from snuba_sdk.expressions import Granularity

from sentry import features, options
from sentry import features
from sentry.eventstore.models import GroupEvent
from sentry.issues.escalating_group_forecast import EscalatingGroupForecast
from sentry.issues.escalating_issues_alg import GroupCount
Expand Down Expand Up @@ -507,13 +506,6 @@ def manage_issue_states(
if updated:
group.status = GroupStatus.UNRESOLVED
group.substatus = GroupSubStatus.ESCALATING
if not options.get("groups.enable-post-update-signal"):
post_save.send(
sender=Group,
instance=group,
created=False,
update_fields=["status", "substatus"],
)
add_group_to_inbox(group, GroupInboxReason.ESCALATING, snooze_details)
record_group_history(group, GroupHistoryStatus.ESCALATING)

Expand Down Expand Up @@ -553,13 +545,6 @@ def manage_issue_states(
if updated:
group.status = GroupStatus.UNRESOLVED
group.substatus = GroupSubStatus.ONGOING
if not options.get("groups.enable-post-update-signal"):
post_save.send(
sender=Group,
instance=group,
created=False,
update_fields=["status", "substatus"],
)
add_group_to_inbox(group, GroupInboxReason.ONGOING, snooze_details)
record_group_history(group, GroupHistoryStatus.ONGOING)

Expand All @@ -574,13 +559,6 @@ def manage_issue_states(
if updated:
group.status = GroupStatus.UNRESOLVED
group.substatus = GroupSubStatus.ONGOING
if not options.get("groups.enable-post-update-signal"):
post_save.send(
sender=Group,
instance=group,
created=False,
update_fields=["status", "substatus"],
)
add_group_to_inbox(group, GroupInboxReason.UNIGNORED, snooze_details)
record_group_history(group, GroupHistoryStatus.UNIGNORED)
Activity.objects.create_group_activity(
Expand Down
12 changes: 0 additions & 12 deletions src/sentry/issues/ongoing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
from typing import Any

import sentry_sdk
from django.db.models.signals import post_save

from sentry import options
from sentry.models.group import Group, GroupStatus
from sentry.models.groupinbox import bulk_remove_groups_from_inbox
from sentry.signals import issue_unresolved
Expand Down Expand Up @@ -53,13 +51,3 @@ def bulk_transition_group_to_ongoing(

with sentry_sdk.start_span(name="bulk_remove_groups_from_inbox"):
bulk_remove_groups_from_inbox(groups_to_transistion)

with sentry_sdk.start_span(name="post_save_send_robust"):
if not options.get("groups.enable-post-update-signal"):
for group in groups_to_transistion:
post_save.send_robust(
sender=Group,
instance=group,
created=False,
update_fields=["status", "substatus"],
)
12 changes: 0 additions & 12 deletions src/sentry/issues/status_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@
from datetime import datetime, timedelta, timezone
from typing import Any

from django.db.models.signals import post_save

from sentry import options
from sentry.integrations.tasks.kick_off_status_syncs import kick_off_status_syncs
from sentry.issues.ignored import IGNORED_CONDITION_FIELDS
from sentry.issues.ongoing import TRANSITION_AFTER_DAYS
Expand Down Expand Up @@ -165,13 +162,4 @@ def handle_status_update(
kick_off_status_syncs.apply_async(
kwargs={"project_id": group.project_id, "group_id": group.id}
)

if not options.get("groups.enable-post-update-signal"):
post_save.send(
sender=Group,
instance=group,
created=False,
update_fields=["status", "substatus"],
)

return ActivityInfo(activity_type, activity_data)
14 changes: 0 additions & 14 deletions src/sentry/models/activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@
from django.conf import settings
from django.db import models
from django.db.models import F
from django.db.models.signals import post_save
from django.utils import timezone

from sentry import options
from sentry.backup.scopes import RelocationScope
from sentry.db.models import (
BoundedPositiveIntegerField,
Expand Down Expand Up @@ -169,26 +167,14 @@ def save(self, *args, **kwargs):

# HACK: support Group.num_comments
if self.type == ActivityType.NOTE.value and self.group is not None:
from sentry.models.group import Group

self.group.update(num_comments=F("num_comments") + 1)
if not options.get("groups.enable-post-update-signal"):
post_save.send_robust(
sender=Group, instance=self.group, created=True, update_fields=["num_comments"]
)

def delete(self, *args, **kwargs):
super().delete(*args, **kwargs)

# HACK: support Group.num_comments
if self.type == ActivityType.NOTE.value and self.group is not None:
from sentry.models.group import Group

self.group.update(num_comments=F("num_comments") - 1)
if not options.get("groups.enable-post-update-signal"):
post_save.send_robust(
sender=Group, instance=self.group, created=True, update_fields=["num_comments"]
)

def send_notification(self):
activity.send_activity_notifications.delay(self.id)
Expand Down
8 changes: 2 additions & 6 deletions src/sentry/models/group.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
from django.utils.translation import gettext_lazy as _
from snuba_sdk import Column, Condition, Op

from sentry import eventstore, eventtypes, options, tagstore
from sentry import eventstore, eventtypes, tagstore
from sentry.backup.scopes import RelocationScope
from sentry.constants import DEFAULT_LOGGER_NAME, LOG_LEVELS, MAX_CULPRIT_LENGTH
from sentry.db.models import (
Expand Down Expand Up @@ -314,11 +314,7 @@ class GroupManager(BaseManager["Group"]):
use_for_related_fields = True

def get_queryset(self):
return (
super()
.get_queryset()
.with_post_update_signal(options.get("groups.enable-post-update-signal"))
)
return super().get_queryset().with_post_update_signal(True)

def by_qualified_short_id(self, organization_id: int, short_id: str):
return self.by_qualified_short_id_bulk(organization_id, [short_id])[0]
Expand Down
11 changes: 2 additions & 9 deletions tests/sentry/api/helpers/test_group_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from sentry.api.helpers.group_index.validators import ValidationError
from sentry.api.issue_search import parse_search_query
from sentry.models.activity import Activity
from sentry.models.group import Group, GroupStatus
from sentry.models.group import GroupStatus
from sentry.models.groupassignee import GroupAssignee
from sentry.models.groupbookmark import GroupBookmark
from sentry.models.grouphash import GroupHash
Expand Down Expand Up @@ -137,8 +137,7 @@ def test_resolving_unresolved_group(self, send_robust: Mock) -> None:
assert send_robust.called

@patch("sentry.signals.issue_ignored.send_robust")
@patch("sentry.issues.status_change.post_save")
def test_ignoring_group_archived_forever(self, post_save: Mock, send_robust: Mock) -> None:
def test_ignoring_group_archived_forever(self, send_robust: Mock) -> None:
group = self.create_group()
add_group_to_inbox(group, GroupInboxReason.NEW)

Expand All @@ -157,12 +156,6 @@ def test_ignoring_group_archived_forever(self, post_save: Mock, send_robust: Moc
assert group.status == GroupStatus.IGNORED
assert group.substatus == GroupSubStatus.FOREVER
assert send_robust.called
post_save.send.assert_called_with(
sender=Group,
instance=group,
created=False,
update_fields=["status", "substatus"],
)
assert not GroupInbox.objects.filter(group=group).exists()

@patch("sentry.signals.issue_ignored.send_robust")
Expand Down
30 changes: 6 additions & 24 deletions tests/sentry/db/models/manager/test_base_query_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from sentry.models.group import Group
from sentry.signals import post_update
from sentry.testutils.cases import TestCase
from sentry.testutils.helpers import override_options


@contextmanager
Expand Down Expand Up @@ -40,31 +39,18 @@ def test_empty_query(self):

class TestSendPostUpdateSignal(TestCase):
def test_not_triggered(self):
with catch_signal(post_update) as handler, override_options(
{"groups.enable-post-update-signal": True}
):
with catch_signal(post_update) as handler:
self.group.message = "hi"
self.group.save()

assert not handler.called

with catch_signal(post_update) as handler, override_options(
{"groups.enable-post-update-signal": True}
):
with catch_signal(post_update) as handler:
self.group.update(message="hi")

assert not handler.called

with catch_signal(post_update) as handler, override_options(
{"groups.enable-post-update-signal": False}
):
assert Group.objects.filter(id=self.group.id).update(message="hi") == 1

assert not handler.called

with catch_signal(post_update) as handler, override_options(
{"groups.enable-post-update-signal": True}
):
with catch_signal(post_update) as handler:
assert (
Group.objects.filter(id=self.group.id)
.with_post_update_signal(False)
Expand All @@ -75,9 +61,7 @@ def test_not_triggered(self):
assert not handler.called

# Test signal not fired when Django detects the query will return no results
with catch_signal(post_update) as handler, override_options(
{"groups.enable-post-update-signal": True}
):
with catch_signal(post_update) as handler:
assert (
Group.objects.filter(id__in=[]).with_post_update_signal(True).update(message="hi")
== 0
Expand All @@ -86,7 +70,7 @@ def test_not_triggered(self):
assert not handler.called

def test_enable(self):
qs = Group.objects.all()
qs = Group.objects.with_post_update_signal(False)
assert not qs._with_post_update_signal
new_qs = qs.with_post_update_signal(True)
# Make sure we don't modify the previous queryset
Expand All @@ -95,9 +79,7 @@ def test_enable(self):

def test_triggered(self):
message = "hi"
with catch_signal(post_update) as handler, override_options(
{"groups.enable-post-update-signal": True}
):
with catch_signal(post_update) as handler:
assert Group.objects.filter(id=self.group.id).update(message=message) == 1

self.group.refresh_from_db()
Expand Down
7 changes: 1 addition & 6 deletions tests/sentry/issues/test_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,12 +224,7 @@ def run_attr_test(
with patch(
"sentry.issues.attributes._log_group_attributes_changed"
) as _log_group_attributes_changed:
with override_options(
{
"groups.enable-post-update-signal": True,
"issues.group_attributes.send_kafka": True,
}
):
with override_options({"issues.group_attributes.send_kafka": True}):
Group.objects.filter(id__in=[g.id for g in groups]).update(**update_fields)
_log_group_attributes_changed.assert_called_with(
Operation.UPDATED, "group", expected_str
Expand Down

0 comments on commit 7c93531

Please sign in to comment.