Browse Source

ref(issues): Add typing to issue status and issue platform files (#66093)

Snigdha Sharma 1 year ago
parent
commit
5afd2e48c4

+ 0 - 2
pyproject.toml

@@ -371,9 +371,7 @@ module = [
     "sentry.issues.endpoints.group_events",
     "sentry.issues.endpoints.organization_group_index",
     "sentry.issues.endpoints.source_map_debug",
-    "sentry.issues.occurrence_consumer",
     "sentry.issues.search",
-    "sentry.issues.status_change",
     "sentry.middleware.access_log",
     "sentry.middleware.auth",
     "sentry.middleware.ratelimit",

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

@@ -597,7 +597,6 @@ def update_groups(
                 acting_user=acting_user,
                 status_details=result.get("statusDetails", {}),
                 sender=update_groups,
-                activity_type=activity_type,
             )
 
     # XXX (ahmed): hack to get the activities to work properly on issues page. Not sure of

+ 11 - 6
src/sentry/issues/occurrence_consumer.py

@@ -8,7 +8,7 @@ from uuid import UUID
 import jsonschema
 import sentry_sdk
 from django.utils import timezone
-from sentry_sdk.tracing import NoOpSpan, Transaction
+from sentry_sdk.tracing import NoOpSpan, Span, Transaction
 
 from sentry import nodestore
 from sentry.event_manager import GroupInfo
@@ -52,7 +52,7 @@ def save_event_from_occurrence(
 
 
 def lookup_event(project_id: int, event_id: str) -> Event:
-    data = nodestore.get(Event.generate_node_id(project_id, event_id))
+    data = nodestore.backend.get(Event.generate_node_id(project_id, event_id))
     if data is None:
         raise EventLookupError(f"Failed to lookup event({event_id}) for project_id({project_id})")
     event = Event(event_id=event_id, project_id=project_id)
@@ -214,8 +214,8 @@ def _get_kwargs(payload: Mapping[str, Any]) -> Mapping[str, Any]:
 
 
 def process_occurrence_message(
-    message: Mapping[str, Any], txn: Transaction | NoOpSpan
-) -> tuple[IssueOccurrence, GroupInfo | None]:
+    message: Mapping[str, Any], txn: Transaction | NoOpSpan | Span
+) -> tuple[IssueOccurrence, GroupInfo | None] | None:
     with metrics.timer("occurrence_consumer._process_message._get_kwargs"):
         kwargs = _get_kwargs(message)
     occurrence_data = kwargs["occurrence_data"]
@@ -260,7 +260,9 @@ def process_occurrence_message(
             return lookup_event_and_process_issue_occurrence(kwargs["occurrence_data"])
 
 
-def _process_message(message: Mapping[str, Any]) -> tuple[IssueOccurrence, GroupInfo | None] | None:
+def _process_message(
+    message: Mapping[str, Any]
+) -> tuple[IssueOccurrence | None, GroupInfo | None] | None:
     """
     :raises InvalidEventPayloadError: when the message is invalid
     :raises EventLookupError: when the provided event_id in the message couldn't be found.
@@ -275,6 +277,9 @@ def _process_message(message: Mapping[str, Any]) -> tuple[IssueOccurrence, Group
             payload_type = message.get("payload_type", PayloadType.OCCURRENCE.value)
             if payload_type == PayloadType.STATUS_CHANGE.value:
                 group = process_status_change_message(message, txn)
+                if not group:
+                    return None
+
                 return None, GroupInfo(group=group, is_new=False, is_regression=False)
             elif payload_type == PayloadType.OCCURRENCE.value:
                 return process_occurrence_message(message, txn)
@@ -287,4 +292,4 @@ def _process_message(message: Mapping[str, Any]) -> tuple[IssueOccurrence, Group
         except (ValueError, KeyError) as e:
             txn.set_tag("result", "error")
             raise InvalidEventPayloadError(e)
-    return
+    return None

+ 6 - 4
src/sentry/issues/status_change.py

@@ -31,18 +31,21 @@ def handle_status_update(
     is_bulk: bool,
     status_details: dict[str, Any],
     acting_user: User | None,
-    activity_type: str | None,
     sender: Any,
 ) -> ActivityInfo:
     """
     Update the status for a list of groups and create entries for Activity and GroupHistory.
+    This currently handles unresolving or ignoring groups.
 
     Returns a tuple of (activity_type, activity_data) for the activity that was created.
     """
     activity_data = {}
+    activity_type = (
+        ActivityType.SET_IGNORED.value
+        if new_status == GroupStatus.IGNORED
+        else ActivityType.SET_UNRESOLVED.value
+    )
     if new_status == GroupStatus.UNRESOLVED:
-        activity_type = ActivityType.SET_UNRESOLVED.value
-
         for group in group_list:
             if group.status == GroupStatus.IGNORED:
                 issue_unignored.send_robust(
@@ -64,7 +67,6 @@ def handle_status_update(
         ignore_duration = (
             status_details.pop("ignoreDuration", None) or status_details.pop("snoozeDuration", None)
         ) or None
-        activity_type = ActivityType.SET_IGNORED.value
         activity_data = {
             "ignoreCount": status_details.get("ignoreCount", None),
             "ignoreDuration": ignore_duration,

+ 2 - 2
src/sentry/issues/status_change_consumer.py

@@ -5,7 +5,7 @@ from collections import defaultdict
 from collections.abc import Iterable, Mapping, Sequence
 from typing import Any
 
-from sentry_sdk.tracing import NoOpSpan, Transaction
+from sentry_sdk.tracing import NoOpSpan, Span, Transaction
 
 from sentry.issues.escalating import manage_issue_states
 from sentry.issues.status_change_message import StatusChangeMessageData
@@ -174,7 +174,7 @@ def _get_status_change_kwargs(payload: Mapping[str, Any]) -> Mapping[str, Any]:
 
 
 def process_status_change_message(
-    message: Mapping[str, Any], txn: Transaction | NoOpSpan
+    message: Mapping[str, Any], txn: Transaction | NoOpSpan | Span
 ) -> Group | None:
     with metrics.timer("occurrence_consumer._process_message.status_change._get_kwargs"):
         kwargs = _get_status_change_kwargs(message)

+ 6 - 0
tests/sentry/issues/test_occurrence_consumer.py

@@ -90,6 +90,7 @@ class IssueOccurrenceProcessMessageTest(IssueOccurrenceTestBase):
             result = _process_message(message)
         assert result is not None
         occurrence = result[0]
+        assert occurrence is not None
 
         fetched_occurrence = IssueOccurrence.fetch(occurrence.id, self.project.id)
         assert fetched_occurrence is not None
@@ -113,6 +114,7 @@ class IssueOccurrenceProcessMessageTest(IssueOccurrenceTestBase):
         assert result is not None
         project_id = event_data["event"]["project_id"]
         occurrence = result[0]
+        assert occurrence is not None
 
         event = eventstore.backend.get_event_by_id(project_id, event_data["event"]["event_id"])
         event = event.for_group(event.group)
@@ -156,6 +158,7 @@ class IssueOccurrenceProcessMessageTest(IssueOccurrenceTestBase):
             result = _process_message(message)
         assert result is not None
         occurrence = result[0]
+        assert occurrence is not None
 
         fetched_occurrence = IssueOccurrence.fetch(occurrence.id, self.project.id)
         assert fetched_occurrence is not None
@@ -177,6 +180,7 @@ class IssueOccurrenceProcessMessageTest(IssueOccurrenceTestBase):
             result = _process_message(message)
         assert result is not None
         occurrence = result[0]
+        assert occurrence is not None
         group = Group.objects.filter(grouphash__hash=occurrence.fingerprint[0]).first()
         assert group.priority == PriorityLevel.LOW
 
@@ -189,6 +193,7 @@ class IssueOccurrenceProcessMessageTest(IssueOccurrenceTestBase):
             result = _process_message(message)
         assert result is not None
         occurrence = result[0]
+        assert occurrence is not None
         group = Group.objects.filter(grouphash__hash=occurrence.fingerprint[0]).first()
         assert group.priority == PriorityLevel.HIGH
 
@@ -224,6 +229,7 @@ class IssueOccurrenceLookupEventIdTest(IssueOccurrenceTestBase):
             processed = _process_message(message)
         assert processed is not None
         occurrence, _ = processed[0], processed[1]
+        assert occurrence is not None
 
         fetched_event = self.eventstore.get_event_by_id(self.project.id, occurrence.event_id)
         assert fetched_event is not None

+ 0 - 4
tests/sentry/issues/test_status_change.py

@@ -33,7 +33,6 @@ class HandleStatusChangeTest(TestCase):
             new_status=GroupStatus.UNRESOLVED,
             new_substatus=GroupSubStatus.ONGOING,
             sender=self,
-            activity_type=None,
         )
 
         assert issue_unignored.called
@@ -59,7 +58,6 @@ class HandleStatusChangeTest(TestCase):
             is_bulk=True,
             status_details={},
             sender=self,
-            activity_type=None,
         )
 
         assert issue_unresolved.called
@@ -85,7 +83,6 @@ class HandleStatusChangeTest(TestCase):
             is_bulk=True,
             status_details={"ignoreDuration": 30},
             sender=self,
-            activity_type=None,
         )
 
         assert issue_ignored.called
@@ -111,7 +108,6 @@ class HandleStatusChangeTest(TestCase):
             is_bulk=True,
             status_details={"ignoreUntilEscalating": True},
             sender=self,
-            activity_type=None,
         )
 
         assert issue_ignored.called

+ 10 - 3
tests/sentry/issues/test_status_change_consumer.py

@@ -42,6 +42,7 @@ class StatusChangeProcessMessageTest(IssueOccurrenceTestBase):
         assert result is not None
 
         self.occurrence = result[0]
+        assert self.occurrence is not None
         self.group = Group.objects.get(grouphash__hash=self.occurrence.fingerprint[0])
         self.fingerprint = ["touch-id"]
 
@@ -168,7 +169,9 @@ class StatusChangeBulkGetGroupsFromFingerprintsTest(IssueOccurrenceTestBase):
             result = _process_message(message)
         assert result is not None
 
-        self.occurrence = result[0]
+        occurrence = result[0]
+        assert occurrence is not None
+        self.occurrence = occurrence
         self.group = Group.objects.get(grouphash__hash=self.occurrence.fingerprint[0])
         self.fingerprint = ["touch-id"]
 
@@ -188,6 +191,7 @@ class StatusChangeBulkGetGroupsFromFingerprintsTest(IssueOccurrenceTestBase):
             result = _process_message(message)
         assert result is not None
         occurrence2 = result[0]
+        assert occurrence2 is not None
         group2 = Group.objects.get(grouphash__hash=occurrence2.fingerprint[0])
 
         # get groups by fingerprint
@@ -211,7 +215,9 @@ class StatusChangeBulkGetGroupsFromFingerprintsTest(IssueOccurrenceTestBase):
         with self.feature("organizations:profile-file-io-main-thread-ingest"):
             result = _process_message(message)
         assert result is not None
-        assert Group.objects.filter(grouphash__hash=result[0].fingerprint[0]).exists()
+        occurrence2 = result[0]
+        assert occurrence2 is not None
+        assert Group.objects.filter(grouphash__hash=occurrence2.fingerprint[0]).exists()
 
         # get groups by fingerprint
         groups_by_fingerprint = bulk_get_groups_from_fingerprints(
@@ -241,7 +247,8 @@ class StatusChangeBulkGetGroupsFromFingerprintsTest(IssueOccurrenceTestBase):
             result = _process_message(message)
         assert result is not None
         occurrence2 = result[0]
-        group2 = Group.objects.get(grouphash__hash=result[0].fingerprint[0], project=project2)
+        assert occurrence2 is not None
+        group2 = Group.objects.get(grouphash__hash=occurrence2.fingerprint[0], project=project2)
 
         assert occurrence2.fingerprint[0] == self.occurrence.fingerprint[0]