Browse Source

chore(issue-priority): Remove issue-priority flag (#72981)

The feature has been GA'd for a few weeks - cleaning up the feature
flag.
Snigdha Sharma 8 months ago
parent
commit
b4a216ca48

+ 1 - 5
src/sentry/api/helpers/group_index/update.py

@@ -188,8 +188,6 @@ def update_groups(
     else:
         group_list = None
 
-    has_priority = False
-
     serializer = None
     # TODO(jess): We may want to look into refactoring GroupValidator
     # to support multiple projects, but this is pretty complicated
@@ -206,8 +204,6 @@ def update_groups(
         )
         if not serializer.is_valid():
             raise serializers.ValidationError(serializer.errors)
-        if not has_priority and features.has("projects:issue-priority", project, actor=user):
-            has_priority = True
 
     if serializer is None:
         return
@@ -258,7 +254,7 @@ def update_groups(
     res_type = None
     activity_type = None
     activity_data: MutableMapping[str, Any | None] | None = None
-    if has_priority and "priority" in result:
+    if "priority" in result:
         handle_priority(
             priority=result["priority"],
             group_list=group_list,

+ 3 - 4
src/sentry/api/serializers/models/group.py

@@ -349,10 +349,9 @@ class GroupSerializerBase(Serializer, ABC):
             "issueCategory": obj.issue_category.name.lower(),
         }
 
-        if features.has("projects:issue-priority", obj.project, actor=None):
-            priority_label = PriorityLevel(obj.priority).to_str() if obj.priority else None
-            group_dict["priority"] = priority_label
-            group_dict["priorityLockedAt"] = obj.priority_locked_at
+        priority_label = PriorityLevel(obj.priority).to_str() if obj.priority else None
+        group_dict["priority"] = priority_label
+        group_dict["priorityLockedAt"] = obj.priority_locked_at
 
         # This attribute is currently feature gated
         if "is_unhandled" in attrs:

+ 6 - 8
src/sentry/event_manager.py

@@ -1990,15 +1990,13 @@ def _create_group(
             extra={"event_id": event.event_id},
         )
 
-    if features.has("projects:issue-priority", project, actor=None):
-        # the kwargs only include priority for non-error issue platform events, which takes precedence.
-        priority = group_creation_kwargs.get("priority", None)
-        if priority is None:
-            priority = _get_priority_for_group(severity, group_creation_kwargs)
-
-        group_creation_kwargs["priority"] = priority
-        group_data["metadata"]["initial_priority"] = priority
+    # the kwargs only include priority for non-error issue platform events, which takes precedence.
+    priority = group_creation_kwargs.get("priority", None)
+    if priority is None:
+        priority = _get_priority_for_group(severity, group_creation_kwargs)
 
+    group_creation_kwargs["priority"] = priority
+    group_data["metadata"]["initial_priority"] = priority
     group_creation_kwargs["data"] = group_data
 
     try:

+ 1 - 8
src/sentry/issues/priority.py

@@ -4,7 +4,6 @@ import logging
 from enum import Enum
 from typing import TYPE_CHECKING
 
-from sentry import features
 from sentry.models.activity import Activity
 from sentry.models.grouphistory import GroupHistoryStatus, record_group_history
 from sentry.models.project import Project
@@ -93,9 +92,6 @@ def get_initial_priority(group: Group) -> PriorityLevel | None:
 
 
 def get_priority_for_ongoing_group(group: Group) -> PriorityLevel | None:
-    if not features.has("projects:issue-priority", group.project, actor=None):
-        return None
-
     # Fall back to the initial priority
     new_priority = get_initial_priority(group)
     if not new_priority:
@@ -112,10 +108,7 @@ def auto_update_priority(group: Group, reason: PriorityChangeReason) -> None:
     """
     Update the priority of a group due to state changes.
     """
-    if (
-        not features.has("projects:issue-priority", group.project, actor=None)
-        or group.priority_locked_at is not None
-    ):
+    if group.priority_locked_at is not None:
         return None
 
     new_priority = None

+ 10 - 14
src/sentry/models/activity.py

@@ -11,7 +11,7 @@ from django.db.models import F
 from django.db.models.signals import post_save
 from django.utils import timezone
 
-from sentry import features, options
+from sentry import options
 from sentry.backup.scopes import RelocationScope
 from sentry.db.models import (
     BoundedPositiveIntegerField,
@@ -40,19 +40,15 @@ class ActivityManager(BaseManager["Activity"]):
     def get_activities_for_group(self, group: Group, num: int) -> Sequence[Activity]:
         activities = []
         activity_qs = self.filter(group=group).order_by("-datetime")
-        initial_priority = None
-
-        if not features.has("projects:issue-priority", group.project):
-            activity_qs = activity_qs.exclude(type=ActivityType.SET_PRIORITY.value)
-        else:
-            # Check if 'initial_priority' is available and the feature flag is on
-            initial_priority_value = group.get_event_metadata().get(
-                "initial_priority", None
-            ) or group.get_event_metadata().get("initial_priority", None)
-
-            initial_priority = (
-                PriorityLevel(initial_priority_value).to_str() if initial_priority_value else None
-            )
+
+        # Check if 'initial_priority' is available
+        initial_priority_value = group.get_event_metadata().get(
+            "initial_priority", None
+        ) or group.get_event_metadata().get("initial_priority", None)
+
+        initial_priority = (
+            PriorityLevel(initial_priority_value).to_str() if initial_priority_value else None
+        )
 
         prev_sig = None
         sig = None

+ 1 - 18
src/sentry/rules/conditions/existing_high_priority_issue.py

@@ -1,11 +1,8 @@
 from collections.abc import Sequence
 from datetime import datetime
 
-from sentry import features
-from sentry.event_manager import HIGH_SEVERITY_THRESHOLD
 from sentry.eventstore.models import GroupEvent
 from sentry.models.activity import Activity
-from sentry.models.group import Group
 from sentry.receivers.rules import has_high_priority_issue_alerts
 from sentry.rules import EventState
 from sentry.rules.conditions.base import EventCondition
@@ -18,16 +15,6 @@ class ExistingHighPriorityIssueCondition(EventCondition):
     id = "sentry.rules.conditions.high_priority_issue.ExistingHighPriorityIssueCondition"
     label = "Sentry marks an existing issue as high priority"
 
-    # This is legacy code that should not be used since "projects:issue-priority" is enabled.
-    # TODO(snigdha): Remove this when the flag is removed.
-    def is_new_high_severity(self, state: EventState, group: Group) -> bool:
-        try:
-            severity = float(group.get_event_metadata().get("severity", ""))
-        except (KeyError, TypeError, ValueError):
-            return False
-
-        return severity >= HIGH_SEVERITY_THRESHOLD
-
     def passes(self, event: GroupEvent, state: EventState) -> bool:
         if not has_high_priority_issue_alerts(self.project):
             return False
@@ -36,11 +23,7 @@ class ExistingHighPriorityIssueCondition(EventCondition):
             return False
 
         is_escalating = state.has_reappeared or state.has_escalated
-        if features.has("projects:issue-priority", self.project):
-            return is_escalating and event.group.priority == PriorityLevel.HIGH
-
-        is_new_high_severity = self.is_new_high_severity(state, event.group)
-        return is_new_high_severity or is_escalating
+        return is_escalating and event.group.priority == PriorityLevel.HIGH
 
     def get_activity(
         self, start: datetime, end: datetime, limit: int

+ 1 - 18
src/sentry/rules/conditions/new_high_priority_issue.py

@@ -1,11 +1,8 @@
 from collections.abc import Sequence
 from datetime import datetime
 
-from sentry import features
-from sentry.event_manager import HIGH_SEVERITY_THRESHOLD
 from sentry.eventstore.models import GroupEvent
 from sentry.models.activity import Activity
-from sentry.models.group import Group
 from sentry.receivers.rules import has_high_priority_issue_alerts
 from sentry.rules import EventState
 from sentry.rules.conditions.base import EventCondition
@@ -18,16 +15,6 @@ class NewHighPriorityIssueCondition(EventCondition):
     id = "sentry.rules.conditions.high_priority_issue.NewHighPriorityIssueCondition"
     label = "Sentry marks a new issue as high priority"
 
-    # This is legacy code that should not be used since "projects:issue-priority" is enabled.
-    # TODO(snigdha): Remove this when the flag is removed.
-    def is_new_high_severity(self, state: EventState, group: Group) -> bool:
-        try:
-            severity = float(group.get_event_metadata().get("severity", ""))
-        except (KeyError, TypeError, ValueError):
-            return False
-
-        return severity >= HIGH_SEVERITY_THRESHOLD
-
     def is_new(self, state: EventState) -> bool:
         if not self.rule or self.rule.environment_id is None:
             return state.is_new
@@ -42,11 +29,7 @@ class NewHighPriorityIssueCondition(EventCondition):
         if not event.project.flags.has_high_priority_alerts:
             return is_new
 
-        if features.has("projects:issue-priority", self.project):
-            return is_new and event.group.priority == PriorityLevel.HIGH
-
-        is_new_high_severity = self.is_new_high_severity(state, event.group)
-        return is_new and is_new_high_severity
+        return is_new and event.group.priority == PriorityLevel.HIGH
 
     def get_activity(
         self, start: datetime, end: datetime, limit: int

+ 0 - 13
tests/sentry/event_manager/test_priority.py

@@ -16,7 +16,6 @@ pytestmark = [requires_snuba]
 
 
 @region_silo_test
-@apply_feature_flag_on_cls("projects:issue-priority")
 @apply_feature_flag_on_cls("projects:first-event-severity-calculation")
 class TestEventManagerPriority(TestCase):
     @patch("sentry.event_manager._get_severity_score", return_value=(0.1121, "ml"))
@@ -30,18 +29,6 @@ class TestEventManagerPriority(TestCase):
         assert event.group.priority == PriorityLevel.HIGH
         assert event.group.get_event_metadata()["initial_priority"] == 75
 
-    @patch("sentry.event_manager._get_severity_score", return_value=(0.1121, "ml"))
-    def test_flag_off(self, mock_get_severity_score: MagicMock):
-        with self.feature({"projects:issue-priority": False}):
-            manager = EventManager(make_event(level=logging.FATAL, platform="python"))
-            event = manager.save(self.project.id)
-
-            mock_get_severity_score.assert_called()
-            assert event.group
-            assert event.group.get_event_metadata()["severity"] == 0.1121
-            assert "initial_priority" not in event.group.get_event_metadata()
-            assert event.group.priority is None
-
     @patch("sentry.event_manager._get_severity_score", return_value=(0.1121, "ml"))
     @patch("sentry.event_manager._get_priority_for_group", return_value=PriorityLevel.HIGH)
     def test_get_priority_for_group_not_called_on_second_event(

+ 0 - 2
tests/sentry/issues/endpoints/test_organization_group_index.py

@@ -5027,7 +5027,6 @@ class GroupUpdateTest(APITestCase, SnubaTestCase):
             group=group2, status=GroupHistoryStatus.UNRESOLVED
         ).exists()
 
-    @with_feature("projects:issue-priority")
     def test_update_priority(self) -> None:
         """
         Bulk-setting priority successfully changes the priority of the groups
@@ -5052,7 +5051,6 @@ class GroupUpdateTest(APITestCase, SnubaTestCase):
                 group=group, type=ActivityType.SET_PRIORITY.value, user_id=self.user.id
             ).exists()
 
-    @with_feature("projects:issue-priority")
     def test_update_priority_no_change(self) -> None:
         """
         When the priority is the same as the current priority, no changes are made

+ 10 - 11
tests/sentry/issues/test_ingest.py

@@ -36,7 +36,6 @@ from sentry.ratelimits.sliding_windows import RequestedQuota
 from sentry.receivers import create_default_projects
 from sentry.snuba.dataset import Dataset
 from sentry.testutils.cases import TestCase
-from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.skips import requires_snuba
 from sentry.types.group import PriorityLevel
 from sentry.utils import json
@@ -129,7 +128,6 @@ class SaveIssueOccurrenceTest(OccurrenceTestMixin, TestCase):
         ):
             save_issue_occurrence(occurrence.to_dict(), event)
 
-    @with_feature("projects:issue-priority")
     def test_new_group_with_default_priority(self) -> None:
         event = self.store_event(data={}, project_id=self.project.id)
         occurrence = self.build_occurrence(event_id=event.event_id)
@@ -137,7 +135,6 @@ class SaveIssueOccurrenceTest(OccurrenceTestMixin, TestCase):
         assert group_info is not None
         assert group_info.group.priority == PriorityLevel.LOW
 
-    @with_feature("projects:issue-priority")
     def test_new_group_with_priority(self) -> None:
         event = self.store_event(data={}, project_id=self.project.id)
         occurrence = self.build_occurrence(
@@ -270,10 +267,13 @@ class SaveIssueFromOccurrenceTest(OccurrenceTestMixin, TestCase):
 
         new_event = self.store_event(data={}, project_id=self.project.id)
         new_occurrence = self.build_occurrence(fingerprint=["another-fingerprint"])
-        with mock.patch("sentry.issues.ingest.metrics") as metrics, mock.patch(
-            "sentry.issues.ingest.issue_rate_limiter.check_and_use_quotas",
-            return_value=[MockGranted(granted=False)],
-        ) as check_and_use_quotas:
+        with (
+            mock.patch("sentry.issues.ingest.metrics") as metrics,
+            mock.patch(
+                "sentry.issues.ingest.issue_rate_limiter.check_and_use_quotas",
+                return_value=[MockGranted(granted=False)],
+            ) as check_and_use_quotas,
+        ):
             assert save_issue_from_occurrence(new_occurrence, new_event, None) is None
             metrics.incr.assert_called_once_with("issues.issue.dropped.rate_limiting")
             assert check_and_use_quotas.call_count == 1
@@ -347,7 +347,6 @@ class SaveIssueFromOccurrenceTest(OccurrenceTestMixin, TestCase):
             metrics_logged = [call.args[0] for call in mock_metrics_incr.mock_calls]
             assert "grouping.in_app_frame_mix" not in metrics_logged
 
-    @with_feature("projects:issue-priority")
     def test_new_group_with_default_priority(self) -> None:
         occurrence = self.build_occurrence()
         event = self.store_event(data={}, project_id=self.project.id)
@@ -355,7 +354,6 @@ class SaveIssueFromOccurrenceTest(OccurrenceTestMixin, TestCase):
         assert group_info is not None
         assert group_info.group.priority == PriorityLevel.LOW
 
-    @with_feature("projects:issue-priority")
     def test_new_group_with_priority(self) -> None:
         occurrence = self.build_occurrence(initial_issue_priority=PriorityLevel.HIGH)
         event = self.store_event(data={}, project_id=self.project.id)
@@ -453,8 +451,9 @@ class SaveIssueOccurrenceToEventstreamTest(OccurrenceTestMixin, TestCase):
         assert group_info is not None
 
         group_event = event.for_group(group_info.group)
-        with mock.patch("sentry.issues.ingest.eventstream") as eventstream, mock.patch.object(
-            event, "for_group", return_value=group_event
+        with (
+            mock.patch("sentry.issues.ingest.eventstream") as eventstream,
+            mock.patch.object(event, "for_group", return_value=group_event),
         ):
             send_issue_occurrence_to_eventstream(event, occurrence, group_info)
             eventstream.backend.insert.assert_called_once_with(

Some files were not shown because too many files changed in this diff