From d8bd0fc06d648200112d4b902f208ccf0133182a Mon Sep 17 00:00:00 2001 From: Mark Story Date: Tue, 26 May 2026 17:35:31 -0400 Subject: [PATCH] chore: Remove Email model The Email model and writes (via signal handlers) was added back in 2017, but there have never been any reads to the table in either sentry, getsentry or internal etl pipelines. This table is often missing records which cause relocation validation failures. Removing this table will reduce our write throughput and simplify the user datamodel. --- fixtures/backup/fresh-install.json | 24 ------- .../backup/user-with-maximum-privileges.json | 8 --- .../backup/user-with-minimum-privileges.json | 8 --- .../backup/user-with-roles-no-superadmin.json | 8 --- .../backup/user-with-superadmin-no-roles.json | 8 --- migrations_lockfile.txt | 2 +- src/sentry/backup/comparators.py | 1 - src/sentry/backup/imports.py | 35 +---------- src/sentry/db/router.py | 1 + .../1101_remove_email_model_pending.py | 29 +++++++++ src/sentry/receivers/__init__.py | 1 - src/sentry/receivers/email.py | 25 -------- src/sentry/users/models/__init__.py | 2 - src/sentry/users/models/email.py | 62 ------------------- src/sentry/users/models/useremail.py | 1 - tests/sentry/backup/test_findings.py | 32 +++++----- 16 files changed, 49 insertions(+), 198 deletions(-) create mode 100644 src/sentry/migrations/1101_remove_email_model_pending.py delete mode 100644 src/sentry/receivers/email.py delete mode 100644 src/sentry/users/models/email.py diff --git a/fixtures/backup/fresh-install.json b/fixtures/backup/fresh-install.json index ac516e86de26bd..a5c3179aea7b48 100644 --- a/fixtures/backup/fresh-install.json +++ b/fixtures/backup/fresh-install.json @@ -39,30 +39,6 @@ "value": "23.6.1" } }, - { - "model": "sentry.email", - "pk": 1, - "fields": { - "email": "admin@example.com", - "date_added": "2023-06-22T22:59:55.531Z" - } - }, - { - "model": "sentry.email", - "pk": 2, - "fields": { - "email": "member@example.com", - "date_added": "2023-06-22T22:59:56.531Z" - } - }, - { - "model": "sentry.email", - "pk": 3, - "fields": { - "email": "superadmin@example.com", - "date_added": "2023-06-22T22:59:57.531Z" - } - }, { "model": "sentry.organization", "pk": 1, diff --git a/fixtures/backup/user-with-maximum-privileges.json b/fixtures/backup/user-with-maximum-privileges.json index eed736f85fe50e..5ee03df2f259e3 100644 --- a/fixtures/backup/user-with-maximum-privileges.json +++ b/fixtures/backup/user-with-maximum-privileges.json @@ -25,14 +25,6 @@ "avatar_url": null } }, - { - "model": "sentry.email", - "pk": 1, - "fields": { - "email": "maximum@example.com", - "date_added": "2023-06-22T22:59:54.521Z" - } - }, { "model": "sentry.authenticator", "pk": 1, diff --git a/fixtures/backup/user-with-minimum-privileges.json b/fixtures/backup/user-with-minimum-privileges.json index 93e6b9d239abe3..496211fa9b218d 100644 --- a/fixtures/backup/user-with-minimum-privileges.json +++ b/fixtures/backup/user-with-minimum-privileges.json @@ -25,14 +25,6 @@ "avatar_url": null } }, - { - "model": "sentry.email", - "pk": 2, - "fields": { - "email": "testing@example.com", - "date_added": "2023-06-22T22:59:54.521Z" - } - }, { "model": "sentry.authenticator", "pk": 2, diff --git a/fixtures/backup/user-with-roles-no-superadmin.json b/fixtures/backup/user-with-roles-no-superadmin.json index 0d2286e5890a53..5979b2838a12cf 100644 --- a/fixtures/backup/user-with-roles-no-superadmin.json +++ b/fixtures/backup/user-with-roles-no-superadmin.json @@ -25,14 +25,6 @@ "avatar_url": null } }, - { - "model": "sentry.email", - "pk": 3, - "fields": { - "email": "testing@example.com", - "date_added": "2023-06-22T22:59:54.521Z" - } - }, { "model": "sentry.authenticator", "pk": 3, diff --git a/fixtures/backup/user-with-superadmin-no-roles.json b/fixtures/backup/user-with-superadmin-no-roles.json index ad463c095a92e4..532cb8b608bc16 100644 --- a/fixtures/backup/user-with-superadmin-no-roles.json +++ b/fixtures/backup/user-with-superadmin-no-roles.json @@ -25,14 +25,6 @@ "avatar_url": null } }, - { - "model": "sentry.email", - "pk": 4, - "fields": { - "email": "testing@example.com", - "date_added": "2023-06-22T22:59:54.521Z" - } - }, { "model": "sentry.authenticator", "pk": 4, diff --git a/migrations_lockfile.txt b/migrations_lockfile.txt index a440e18f04a483..d02269d4e3e0d8 100644 --- a/migrations_lockfile.txt +++ b/migrations_lockfile.txt @@ -31,7 +31,7 @@ replays: 0007_organizationmember_replay_access seer: 0017_drop_old_fk_columns -sentry: 1100_add_relocation_file_bucket_path +sentry: 1101_remove_email_model_pending social_auth: 0003_social_auth_json_field diff --git a/src/sentry/backup/comparators.py b/src/sentry/backup/comparators.py index 35b8d40e82ffc1..c7afaf0fc6c49b 100644 --- a/src/sentry/backup/comparators.py +++ b/src/sentry/backup/comparators.py @@ -879,7 +879,6 @@ def get_default_comparators() -> dict[str, list[JSONScrubbingComparator]]: "sentry.dashboardwidgetqueryondemand": [DateUpdatedComparator("date_modified")], "sentry.dashboardwidgetquery": [DateUpdatedComparator("date_modified")], "sentry.dashboardfieldlink": [DateUpdatedComparator("date_added", "date_updated")], - "sentry.email": [DateUpdatedComparator("date_added")], "sentry.organization": [AutoSuffixComparator("slug")], "sentry.organizationintegration": [DateUpdatedComparator("date_updated")], "sentry.organizationmember": [ diff --git a/src/sentry/backup/imports.py b/src/sentry/backup/imports.py index 1248c2e9e92d43..31b7a41bfce89d 100644 --- a/src/sentry/backup/imports.py +++ b/src/sentry/backup/imports.py @@ -129,7 +129,6 @@ def _import( # Import here to prevent circular module resolutions. from sentry.models.organization import Organization from sentry.models.organizationmember import OrganizationMember - from sentry.users.models.email import Email from sentry.users.models.user import User if SiloMode.get_current_mode() == SiloMode.CONTROL: @@ -144,7 +143,6 @@ def _import( flags = flags._replace(import_uuid=uuid4().hex) deps = dependencies() - user_model_name = get_model_name(User) org_auth_token_model_name = get_model_name(OrgAuthToken) org_member_model_name = get_model_name(OrganizationMember) org_model_name = get_model_name(Organization) @@ -181,10 +179,6 @@ def _import( if filter_by is not None: filters.append(filter_by) - # `sentry.Email` models don't have any explicit dependencies on `sentry.User`, so we need to - # find and record them manually. - user_to_email = dict() - if filter_by.model == Organization: # To properly filter organizations, we need to grab their users first. There is no # elegant way to do this: we'll just have to read the import JSON until we get to the @@ -203,12 +197,7 @@ def _import( for obj in serializers.deserialize("json", content): o = obj.object model_name = get_model_name(o) - if model_name == user_model_name: - username = getattr(o, "username", None) - email = getattr(o, "email", None) - if username is not None and email is not None: - user_to_email[username] = email - elif model_name == org_model_name: + if model_name == org_model_name: pk = getattr(o, "pk", None) slug = getattr(o, "slug", None) if pk is not None and slug in filter_by.values: @@ -224,30 +213,10 @@ def _import( # org member we're going to see. We can ignore the rest of the models. break elif filter_by.model == User: - seen_first_user_model = False - for obj in serializers.deserialize("json", content): - o = obj.object - model_name = get_model_name(o) - if model_name == user_model_name: - seen_first_user_model = False - username = getattr(o, "username", None) - email = getattr(o, "email", None) - if username is not None and email is not None: - user_to_email[username] = email - elif seen_first_user_model: - break + pass else: raise TypeError("Filter arguments must only apply to `Organization` or `User` models") - user_filter = next(f for f in filters if f.model == User) - email_filter = Filter[str]( - model=Email, - field="email", - values={v for k, v in user_to_email.items() if k in user_filter.values}, - ) - - filters.append(email_filter) - # Reorder models according to dependencies to ensure correct import order. # This is necessary for backward compatibility with exports created before # __relocation_dependencies__ was added to models like DataSource. diff --git a/src/sentry/db/router.py b/src/sentry/db/router.py index e87cdf33c283f5..fa7ce2173bec21 100644 --- a/src/sentry/db/router.py +++ b/src/sentry/db/router.py @@ -82,6 +82,7 @@ class SiloRouter: "sentry_dashboardtombstone": SiloMode.CELL, "sentry_dashboardwidgetsnapshot": SiloMode.CELL, "sentry_datasecrecywaiver": SiloMode.CELL, + "sentry_email": SiloMode.CELL, "sentry_incidentseen": SiloMode.CELL, "sentry_incidentsubscription": SiloMode.CELL, "sentry_incidenttrigger": SiloMode.CELL, diff --git a/src/sentry/migrations/1101_remove_email_model_pending.py b/src/sentry/migrations/1101_remove_email_model_pending.py new file mode 100644 index 00000000000000..032b402b3360b7 --- /dev/null +++ b/src/sentry/migrations/1101_remove_email_model_pending.py @@ -0,0 +1,29 @@ +# Generated by Django 5.2.14 on 2026-05-26 20:43 + +from sentry.new_migrations.migrations import CheckedMigration +from sentry.new_migrations.monkey.models import SafeDeleteModel +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", "1100_add_relocation_file_bucket_path"), + ] + + operations = [ + SafeDeleteModel(name="Email", deletion_action=DeletionAction.MOVE_TO_PENDING), + ] diff --git a/src/sentry/receivers/__init__.py b/src/sentry/receivers/__init__.py index e9af536ddc46a4..281623063670e8 100644 --- a/src/sentry/receivers/__init__.py +++ b/src/sentry/receivers/__init__.py @@ -2,7 +2,6 @@ from .auth import * # noqa: F401,F403 from .core import * # noqa: F401,F403 from .data_forwarding import * # noqa: F401,F403 -from .email import * # noqa: F401,F403 from .experiments import * # noqa: F401,F403 from .features import * # noqa: F401,F403 from .onboarding import * # noqa: F401,F403 diff --git a/src/sentry/receivers/email.py b/src/sentry/receivers/email.py deleted file mode 100644 index f074a9889a69e5..00000000000000 --- a/src/sentry/receivers/email.py +++ /dev/null @@ -1,25 +0,0 @@ -from django.db import IntegrityError, router, transaction -from django.db.models.signals import post_delete, post_save - -from sentry.users.models.email import Email -from sentry.users.models.useremail import UserEmail - - -def create_email(instance, created, **kwargs): - if created: - try: - with transaction.atomic(router.db_for_write(Email)): - Email.objects.create(email=instance.email) - except IntegrityError: - pass - - -def delete_email(instance, **kwargs): - if UserEmail.objects.filter(email__iexact=instance.email).exists(): - return - - Email.objects.filter(email=instance.email).delete() - - -post_save.connect(create_email, sender=UserEmail, dispatch_uid="create_email", weak=False) -post_delete.connect(delete_email, sender=UserEmail, dispatch_uid="delete_email", weak=False) diff --git a/src/sentry/users/models/__init__.py b/src/sentry/users/models/__init__.py index 03db9e11812b97..eca55526ff7e07 100644 --- a/src/sentry/users/models/__init__.py +++ b/src/sentry/users/models/__init__.py @@ -1,5 +1,4 @@ from sentry.users.models.authenticator import Authenticator -from sentry.users.models.email import Email from sentry.users.models.identity import Identity from sentry.users.models.lostpasswordhash import LostPasswordHash from sentry.users.models.user import User @@ -11,7 +10,6 @@ __all__ = ( "Authenticator", - "Email", "Identity", "LostPasswordHash", "User", diff --git a/src/sentry/users/models/email.py b/src/sentry/users/models/email.py deleted file mode 100644 index 68957bd78b0a8b..00000000000000 --- a/src/sentry/users/models/email.py +++ /dev/null @@ -1,62 +0,0 @@ -from __future__ import annotations - -from django.db import models -from django.forms import model_to_dict -from django.utils import timezone -from django.utils.translation import gettext_lazy as _ - -from sentry.backup.dependencies import ImportKind, PrimaryKeyMap, get_model_name -from sentry.backup.helpers import ImportFlags -from sentry.backup.scopes import ImportScope, RelocationScope -from sentry.db.models import CIEmailField, Model, control_silo_model, sane_repr - - -@control_silo_model -class Email(Model): - """ - Email represents a unique email. Email settings (unsubscribe state) should be associated here. - UserEmail represents whether a given user account has access to that email. - """ - - __relocation_scope__ = RelocationScope.Excluded - __relocation_dependencies__ = {"sentry.User"} - __relocation_custom_ordinal__ = ["email"] - - email = CIEmailField(_("email address"), unique=True, max_length=200) - date_added = models.DateTimeField(default=timezone.now) - - class Meta: - app_label = "sentry" - db_table = "sentry_email" - - __repr__ = sane_repr("email") - - @classmethod - def query_for_relocation_export(cls, q: models.Q, pk_map: PrimaryKeyMap) -> models.Q: - from sentry.users.models.user import User - from sentry.users.models.useremail import UserEmail - - # `Sentry.Email` models don't have any explicit dependencies on `Sentry.User`, so we need to - # find them manually via `UserEmail`. - emails = ( - UserEmail.objects.filter(user_id__in=pk_map.get_pks(get_model_name(User))) - .annotate(email_lower=models.Func(models.F("email"), function="LOWER")) - .values_list("email_lower", flat=True) - ) - - # Use case-insensitive matching as useremail and email are case-insensitive - # This doesn't handle upper case Email records, with lowercase Useremail records. - return q & models.Q(email__in=emails) - - def write_relocation_import( - self, _s: ImportScope, _f: ImportFlags - ) -> tuple[int, ImportKind] | None: - # Ensure that we never attempt to duplicate email entries, as they must always be unique. - (email, created) = self.__class__.objects.get_or_create( - email=self.email, defaults=model_to_dict(self) - ) - if email: - self.pk = email.pk - self.save() - - return (self.pk, ImportKind.Inserted if created else ImportKind.Existing) diff --git a/src/sentry/users/models/useremail.py b/src/sentry/users/models/useremail.py index ec375d5c426e53..451fb875fd4d97 100644 --- a/src/sentry/users/models/useremail.py +++ b/src/sentry/users/models/useremail.py @@ -55,7 +55,6 @@ def get_primary_email(self, user: RpcUser | User) -> UserEmail: @control_silo_model class UserEmail(ControlOutboxProducingModel): __relocation_scope__ = RelocationScope.User - __relocation_dependencies__ = {"sentry.Email"} __relocation_custom_ordinal__ = ["user", "email"] user = FlexibleForeignKey(settings.AUTH_USER_MODEL, related_name="emails") diff --git a/tests/sentry/backup/test_findings.py b/tests/sentry/backup/test_findings.py index 83c610cdc974fa..28656c828415a4 100644 --- a/tests/sentry/backup/test_findings.py +++ b/tests/sentry/backup/test_findings.py @@ -20,7 +20,7 @@ RpcImportErrorKind, ) from sentry.testutils.cases import TestCase -from sentry.users.models.email import Email +from sentry.users.models.useremail import UserEmail encoder = FindingJSONEncoder( sort_keys=True, @@ -64,7 +64,7 @@ def to_dict(self) -> dict[str, Any]: class FindingsTests(TestCase): def test_defaults(self) -> None: finding = TestFinding( - on=InstanceID(model=str(get_model_name(Email))), + on=InstanceID(model=str(get_model_name(UserEmail))), reason="test reason", ) @@ -75,7 +75,7 @@ def test_defaults(self) -> None: "kind": "Unknown", "left_pk": null, "on": { - "model": "sentry.email", + "model": "sentry.useremail", "ordinal": null }, "reason": "test reason", @@ -86,7 +86,7 @@ def test_defaults(self) -> None: finding.pretty() == """TestFinding( kind: Unknown, - on: InstanceID(model: 'sentry.email'), + on: InstanceID(model: 'sentry.useremail'), reason: test reason )""" ) @@ -94,7 +94,7 @@ def test_defaults(self) -> None: def test_no_nulls(self) -> None: finding = TestFinding( kind=TestFindingKind.Foo, - on=InstanceID(model=str(get_model_name(Email)), ordinal=1), + on=InstanceID(model=str(get_model_name(UserEmail)), ordinal=1), left_pk=2, right_pk=3, reason="test reason", @@ -106,7 +106,7 @@ def test_no_nulls(self) -> None: "kind": "Foo", "left_pk": 2, "on": { - "model": "sentry.email", + "model": "sentry.useremail", "ordinal": 1 }, "reason": "test reason", @@ -117,7 +117,7 @@ def test_no_nulls(self) -> None: finding.pretty() == """TestFinding( kind: Foo, - on: InstanceID(model: 'sentry.email', ordinal: 1), + on: InstanceID(model: 'sentry.useremail', ordinal: 1), left_pk: 2, right_pk: 3, reason: test reason @@ -127,7 +127,7 @@ def test_no_nulls(self) -> None: def test_comparator_finding(self) -> None: finding = ComparatorFinding( kind=ComparatorFindingKind.Unknown, - on=InstanceID(model=str(get_model_name(Email)), ordinal=1), + on=InstanceID(model=str(get_model_name(UserEmail)), ordinal=1), left_pk=2, right_pk=3, reason="test reason", @@ -139,7 +139,7 @@ def test_comparator_finding(self) -> None: "kind": "Unknown", "left_pk": 2, "on": { - "model": "sentry.email", + "model": "sentry.useremail", "ordinal": 1 }, "reason": "test reason", @@ -150,7 +150,7 @@ def test_comparator_finding(self) -> None: finding.pretty() == """ComparatorFinding( kind: Unknown, - on: InstanceID(model: 'sentry.email', ordinal: 1), + on: InstanceID(model: 'sentry.useremail', ordinal: 1), left_pk: 2, right_pk: 3, reason: test reason @@ -160,7 +160,7 @@ def test_comparator_finding(self) -> None: def test_rpc_export_error(self) -> None: finding = RpcExportError( kind=RpcExportErrorKind.Unknown, - on=InstanceID(model=str(get_model_name(Email)), ordinal=1), + on=InstanceID(model=str(get_model_name(UserEmail)), ordinal=1), left_pk=2, right_pk=3, reason="test reason", @@ -172,7 +172,7 @@ def test_rpc_export_error(self) -> None: "kind": "Unknown", "left_pk": 2, "on": { - "model": "sentry.email", + "model": "sentry.useremail", "ordinal": 1 }, "reason": "test reason", @@ -183,7 +183,7 @@ def test_rpc_export_error(self) -> None: finding.pretty() == """RpcExportError( kind: Unknown, - on: InstanceID(model: 'sentry.email', ordinal: 1), + on: InstanceID(model: 'sentry.useremail', ordinal: 1), left_pk: 2, right_pk: 3, reason: test reason @@ -193,7 +193,7 @@ def test_rpc_export_error(self) -> None: def test_rpc_import_error(self) -> None: finding = RpcImportError( kind=RpcImportErrorKind.Unknown, - on=InstanceID(model=str(get_model_name(Email)), ordinal=1), + on=InstanceID(model=str(get_model_name(UserEmail)), ordinal=1), left_pk=2, right_pk=3, reason="test reason", @@ -205,7 +205,7 @@ def test_rpc_import_error(self) -> None: "kind": "Unknown", "left_pk": 2, "on": { - "model": "sentry.email", + "model": "sentry.useremail", "ordinal": 1 }, "reason": "test reason", @@ -216,7 +216,7 @@ def test_rpc_import_error(self) -> None: finding.pretty() == """RpcImportError( kind: Unknown, - on: InstanceID(model: 'sentry.email', ordinal: 1), + on: InstanceID(model: 'sentry.useremail', ordinal: 1), left_pk: 2, right_pk: 3, reason: test reason