diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index 8110b79486af6a..c2216bf72cc682 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -15,7 +15,7 @@ remote_subscriptions: 0003_drop_remote_subscription replays: 0004_index_together -sentry: 0804_delete_metrics_key_indexer_pt2 +sentry: 0805_remove_monitor_attachment_id_pt1 social_auth: 0002_default_auto_field diff --git a/src/sentry/migrations/0805_remove_monitor_attachment_id_pt1.py b/src/sentry/migrations/0805_remove_monitor_attachment_id_pt1.py new file mode 100644 index 00000000000000..ac38a0715b687a --- /dev/null +++ b/src/sentry/migrations/0805_remove_monitor_attachment_id_pt1.py @@ -0,0 +1,33 @@ +# Generated by Django 5.1.4 on 2025-01-03 21:24 + +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.fields import SafeRemoveField +from sentry.new_migrations.monkey.state import DeletionAction + + +class Migration(CheckedMigration): + # This flag is used to mark that a migration shouldn't be automatically run in production. + # This should only be used for operations where it's safe to run the migration after your + # code has deployed. So this should not be used for most operations that alter the schema + # of a table. + # Here are some things that make sense to mark as post deployment: + # - Large data migrations. Typically we want these to be run manually so that they can be + # monitored and not block the deploy for a long period of time while they run. + # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to + # run this outside deployments so that we don't block them. Note that while adding an index + # is a schema change, it's completely safe to run the operation after the code has deployed. + # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment + + is_post_deployment = False + + dependencies = [ + ("sentry", "0804_delete_metrics_key_indexer_pt2"), + ] + + operations = [ + SafeRemoveField( + model_name="monitorcheckin", + name="attachment_id", + deletion_action=DeletionAction.MOVE_TO_PENDING, + ) + ] diff --git a/src/sentry/monitors/models.py b/src/sentry/monitors/models.py index 511b6019b5b3e8..0af154d01fbf19 100644 --- a/src/sentry/monitors/models.py +++ b/src/sentry/monitors/models.py @@ -12,7 +12,7 @@ from django.conf import settings from django.db import models from django.db.models import Case, IntegerField, Q, Value, When -from django.db.models.signals import post_delete, pre_save +from django.db.models.signals import pre_save from django.dispatch import receiver from django.utils import timezone @@ -533,8 +533,6 @@ class MonitorCheckIn(Model): that occurred during the check-in. """ - attachment_id = BoundedBigIntegerField(null=True) - objects: ClassVar[BaseManager[Self]] = BaseManager(cache_fields=("guid",)) class Meta: @@ -584,16 +582,6 @@ def _update_timestamps(self): pass -def delete_file_for_monitorcheckin(instance: MonitorCheckIn, **kwargs): - if file_id := instance.attachment_id: - from sentry.models.files import File - - File.objects.filter(id=file_id).delete() - - -post_delete.connect(delete_file_for_monitorcheckin, sender=MonitorCheckIn) - - @region_silo_model class MonitorLocation(Model): __relocation_scope__ = RelocationScope.Excluded diff --git a/tests/sentry/monitors/endpoints/test_base_monitor_checkin_attachment.py b/tests/sentry/monitors/endpoints/test_base_monitor_checkin_attachment.py deleted file mode 100644 index c7941ea0ee96f7..00000000000000 --- a/tests/sentry/monitors/endpoints/test_base_monitor_checkin_attachment.py +++ /dev/null @@ -1,70 +0,0 @@ -from django.core.files.base import ContentFile - -from sentry.db.postgres.transactions import in_test_hide_transaction_boundary -from sentry.models.files import File -from sentry.monitors.models import CheckInStatus, MonitorCheckIn -from sentry.testutils.cases import MonitorTestCase - - -class BaseMonitorCheckInAttachmentEndpointTest(MonitorTestCase): - __test__ = False - - def setUp(self): - super().setUp() - self.login_as(self.user) - - def test_download(self): - file = self.create_file(name="log.txt", type="checkin.attachment") - file.putfile(ContentFile(b"some data!")) - - monitor = self._create_monitor() - monitor_environment = self._create_monitor_environment(monitor) - checkin = MonitorCheckIn.objects.create( - monitor=monitor, - monitor_environment=monitor_environment, - project_id=self.project.id, - date_added=monitor.date_added, - status=CheckInStatus.IN_PROGRESS, - attachment_id=file.id, - ) - - resp = self.get_success_response(self.organization.slug, monitor.slug, checkin.guid) - assert resp.get("Content-Disposition") == "attachment; filename=log.txt" - assert b"".join(resp.streaming_content) == b"some data!" - - def test_download_no_file(self): - monitor = self._create_monitor() - monitor_environment = self._create_monitor_environment(monitor) - checkin = MonitorCheckIn.objects.create( - monitor=monitor, - monitor_environment=monitor_environment, - project_id=self.project.id, - date_added=monitor.date_added, - status=CheckInStatus.IN_PROGRESS, - ) - - resp = self.get_error_response( - self.organization.slug, monitor.slug, checkin.guid, status_code=404 - ) - assert resp.data["detail"] == "Check-in has no attachment" - - def test_delete_cascade(self): - file = self.create_file(name="log.txt", type="checkin.attachment") - file.putfile(ContentFile(b"some data!")) - - monitor = self._create_monitor() - monitor_environment = self._create_monitor_environment(monitor) - checkin = MonitorCheckIn.objects.create( - monitor=monitor, - monitor_environment=monitor_environment, - project_id=self.project.id, - date_added=monitor.date_added, - status=CheckInStatus.IN_PROGRESS, - attachment_id=file.id, - ) - - # checkin has a post_delete signal that removes files. - with in_test_hide_transaction_boundary(): - checkin.delete() - - assert not File.objects.filter(type="checkin.attachment").exists()