Browse Source

ref: Deprecate group_id field in EventAttachment (#14167)

Since we never actually return the group ID from the EventAttachment table
 or use it for lookup we shouldn't store it there.

 As we migrate event storage fully to Snuba, getting this field
 populated when we store the attachment will be more complicated
 since it requires a lookup to the Event table to get the group
 ID and there'll be a race condition between the event and the event
 attachment being saved.
Lyn Nagara 5 years ago
parent
commit
b817af3f51

+ 0 - 1
bin/load-mocks

@@ -597,7 +597,6 @@ def main(num_events=1, extra_events=False):
 
                 EventAttachment.objects.create(
                     project_id=project.id,
-                    group_id=event1.group_id,
                     event_id=event1.event_id,
                     name='example-logfile.txt',
                     file=File.objects.get_or_create(

+ 0 - 1
src/sentry/deletions/defaults/group.py

@@ -30,7 +30,6 @@ class GroupDeletionTask(ModelDeletionTask):
             models.GroupEmailThread,
             models.GroupSubscription,
             models.UserReport,
-            models.EventAttachment,
             IncidentGroup,
             # Event is last as its the most time consuming
             models.Event,

+ 1 - 9
src/sentry/event_manager.py

@@ -39,7 +39,7 @@ from sentry.models import (
     Activity, Environment, Event, EventDict, EventError, EventMapping, EventUser, Group,
     GroupEnvironment, GroupHash, GroupLink, GroupRelease, GroupResolution, GroupStatus,
     Project, Release, ReleaseEnvironment, ReleaseProject,
-    ReleaseProjectEnvironment, UserReport, Organization, EventAttachment,
+    ReleaseProjectEnvironment, UserReport, Organization,
 )
 from sentry.plugins import plugins
 from sentry.signals import event_discarded, event_saved, first_event_received
@@ -836,14 +836,6 @@ class EventManager(object):
                 environment=environment,
             )
 
-            # Update any event attachment that arrived before the event group was defined.
-            EventAttachment.objects.filter(
-                project_id=project.id,
-                event_id=event_id,
-            ).update(
-                group_id=group.id,
-            )
-
         # save the event unless its been sampled
         if not is_sample:
             try:

+ 1 - 0
src/sentry/models/eventattachment.py

@@ -10,6 +10,7 @@ class EventAttachment(Model):
     __core__ = False
 
     project_id = BoundedBigIntegerField()
+    # TODO(lyn): group_id is now deprecated, drop this
     group_id = BoundedBigIntegerField(null=True, db_index=True)
     event_id = models.CharField(max_length=32, db_index=True)
     file = FlexibleForeignKey('sentry.File')

+ 0 - 1
src/sentry/tasks/store.py

@@ -436,7 +436,6 @@ def save_attachment(event, attachment):
 
     EventAttachment.objects.create(
         event_id=event.event_id,
-        group_id=event.group_id,
         project_id=event.project_id,
         name=attachment.name,
         file=file,

+ 0 - 1
src/sentry/testutils/factories.py

@@ -669,7 +669,6 @@ class Factories(object):
 
         return EventAttachment.objects.create(
             project_id=event.project_id,
-            group_id=event.group_id,
             event_id=event.event_id,
             file=file,
             **kwargs

+ 2 - 21
src/sentry/web/api.py

@@ -47,7 +47,7 @@ from sentry.lang.native.minidump import (
     merge_attached_event, merge_attached_breadcrumbs, write_minidump_placeholder,
     MINIDUMP_ATTACHMENT_TYPE,
 )
-from sentry.models import Project, File, EventAttachment, Event
+from sentry.models import Project, File, EventAttachment
 from sentry.signals import (
     event_accepted, event_dropped, event_filtered, event_received,
 )
@@ -654,32 +654,13 @@ class EventAttachmentStoreView(StoreView):
             )
             file.putfile(uploaded_file)
 
-            # To avoid a race with EventManager which tries to set the group_id on attachments received before
-            # the event, first insert the attachment, then lookup for the event for its group.
-            event_attachment = EventAttachment.objects.create(
+            EventAttachment.objects.create(
                 project_id=project_id,
                 event_id=event_id,
                 name=uploaded_file.name,
                 file=file,
             )
 
-            try:
-                event = Event.objects.get(
-                    project_id=project_id,
-                    event_id=event_id,
-                )
-            except Event.DoesNotExist:
-                pass
-            else:
-                # If event was created but the group not defined, EventManager will take care of setting the
-                # group to all dangling attachments
-                if event.group_id is not None:
-                    EventAttachment.objects.filter(
-                        id=event_attachment.id,
-                    ).update(
-                        group_id=event.group_id,
-                    )
-
         return HttpResponse(status=201)
 
 

+ 0 - 1
tests/sentry/api/endpoints/test_event_attachment_details.py

@@ -32,7 +32,6 @@ class CreateAttachmentMixin(object):
 
         self.attachment = EventAttachment.objects.create(
             event_id=self.event.event_id,
-            group_id=self.event.group_id,
             project_id=self.event.project_id,
             file=self.file,
             name='hello.png',

+ 0 - 2
tests/sentry/api/endpoints/test_event_attachments.py

@@ -31,7 +31,6 @@ class EventAttachmentsTest(APITestCase):
 
         attachment1 = EventAttachment.objects.create(
             event_id=event1.event_id,
-            group_id=event1.group_id,
             project_id=event1.project_id,
             file=File.objects.create(
                 name='hello.png',
@@ -42,7 +41,6 @@ class EventAttachmentsTest(APITestCase):
 
         EventAttachment.objects.create(
             event_id=event2.event_id,
-            group_id=event2.group_id,
             project_id=event2.project_id,
             file=File.objects.create(
                 name='hello.png',

+ 1 - 15
tests/sentry/deletions/test_group.py

@@ -5,7 +5,7 @@ from uuid import uuid4
 from sentry import tagstore
 from sentry.tagstore.models import EventTag
 from sentry.models import (
-    Event, EventAttachment, EventMapping, File, Group, GroupAssignee, GroupHash, GroupMeta, GroupRedirect,
+    Event, EventMapping, Group, GroupAssignee, GroupHash, GroupMeta, GroupRedirect,
     ScheduledDeletion, UserReport
 )
 from sentry.tasks.deletion import run_deletion
@@ -24,16 +24,6 @@ class DeleteGroupTest(TestCase):
             event_id='a' * 32,
             group_id=group.id,
         )
-        EventAttachment.objects.create(
-            event_id=event.event_id,
-            group_id=event.group_id,
-            project_id=event.project_id,
-            file=File.objects.create(
-                name='hello.png',
-                type='image/png',
-            ),
-            name='hello.png',
-        )
         UserReport.objects.create(
             group_id=group.id,
             project_id=event.project_id,
@@ -88,10 +78,6 @@ class DeleteGroupTest(TestCase):
             run_deletion(deletion.id)
 
         assert not Event.objects.filter(id=event.id).exists()
-        assert not EventAttachment.objects.filter(
-            event_id=event.event_id,
-            group_id=group.id,
-        ).exists()
         assert not EventMapping.objects.filter(
             group_id=group.id,
         ).exists()

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