Browse Source

chore(hybrid-cloud): Release.user => user_id in queries. (#44405)

Builds off https://github.com/getsentry/sentry/pull/44391

The goal is to break Release.user foreign key in production,. In follow
PRs I will introduce the migration. This change makes all queries that
depended on the relation use the identifier directly instead.

---------

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Zach Collins 2 years ago
parent
commit
8201f4f576

+ 1 - 1
src/sentry/api/endpoints/organization_release_details.py

@@ -477,7 +477,7 @@ class OrganizationReleaseDetailsEndpoint(
                     )
                 fetch_commits = not commit_list
                 try:
-                    release.set_refs(refs, request.user, fetch=fetch_commits)
+                    release.set_refs(refs, request.user.id, fetch=fetch_commits)
                 except InvalidRepository as e:
                     scope.set_tag("failure_reason", "InvalidRepository")
                     return Response({"refs": [str(e)]}, status=400)

+ 8 - 3
src/sentry/api/endpoints/organization_releases.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import re
 from datetime import datetime, timedelta
 
@@ -281,7 +283,7 @@ class OrganizationReleasesEndpoint(
             else:
                 queryset = queryset.filter(status=status_int)
 
-        queryset = queryset.select_related("owner").annotate(date=F("date_added"))
+        queryset = queryset.annotate(date=F("date_added"))
 
         queryset = add_environment_to_queryset(queryset, filter_params)
         if query:
@@ -462,6 +464,9 @@ class OrganizationReleasesEndpoint(
                     projects.append(allowed_projects[slug])
 
                 new_status = result.get("status")
+                owner_id: int | None = None
+                if owner := result.get("owner"):
+                    owner_id = owner.id
 
                 # release creation is idempotent to simplify user
                 # experiences
@@ -472,7 +477,7 @@ class OrganizationReleasesEndpoint(
                         defaults={
                             "ref": result.get("ref"),
                             "url": result.get("url"),
-                            "owner": result.get("owner"),
+                            "owner_id": owner_id,
                             "date_released": result.get("dateReleased"),
                             "status": new_status or ReleaseStatus.OPEN,
                             "user_agent": request.META.get("HTTP_USER_AGENT", "")[:256],
@@ -537,7 +542,7 @@ class OrganizationReleasesEndpoint(
                         )
                     fetch_commits = not commit_list
                     try:
-                        release.set_refs(refs, request.user, fetch=fetch_commits)
+                        release.set_refs(refs, request.user.id, fetch=fetch_commits)
                     except InvalidRepository as e:
                         scope.set_tag("failure_reason", "InvalidRepository")
                         return Response({"refs": [str(e)]}, status=400)

+ 11 - 9
src/sentry/api/endpoints/project_releases.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from django.db import IntegrityError, transaction
 from django.db.models import Q
 from rest_framework.request import Request
@@ -45,14 +47,10 @@ class ProjectReleasesEndpoint(ProjectEndpoint, EnvironmentMixin):
             queryset = Release.objects.none()
             environment = None
         else:
-            queryset = (
-                Release.objects.filter(
-                    projects=project,
-                    organization_id=project.organization_id,
-                )
-                .filter(Q(status=ReleaseStatus.OPEN) | Q(status=None))
-                .select_related("owner")
-            )
+            queryset = Release.objects.filter(
+                projects=project,
+                organization_id=project.organization_id,
+            ).filter(Q(status=ReleaseStatus.OPEN) | Q(status=None))
             if environment is not None:
                 queryset = queryset.filter(
                     releaseprojectenvironment__project=project,
@@ -120,6 +118,10 @@ class ProjectReleasesEndpoint(ProjectEndpoint, EnvironmentMixin):
 
                 # release creation is idempotent to simplify user
                 # experiences
+                owner_id: int | None = None
+                if owner := result.get("owner"):
+                    owner_id = owner.id
+
                 try:
                     with transaction.atomic():
                         release, created = (
@@ -128,7 +130,7 @@ class ProjectReleasesEndpoint(ProjectEndpoint, EnvironmentMixin):
                                 version=result["version"],
                                 ref=result.get("ref"),
                                 url=result.get("url"),
-                                owner=result.get("owner"),
+                                owner_id=owner_id,
                                 date_released=result.get("dateReleased"),
                                 status=new_status or ReleaseStatus.OPEN,
                                 user_agent=request.META.get("HTTP_USER_AGENT", ""),

+ 2 - 2
src/sentry/models/release.py

@@ -806,7 +806,7 @@ class Release(Model):
             if COMMIT_RANGE_DELIMITER in ref["commit"]:
                 ref["previousCommit"], ref["commit"] = ref["commit"].split(COMMIT_RANGE_DELIMITER)
 
-    def set_refs(self, refs, user, fetch=False):
+    def set_refs(self, refs, user_id, fetch=False):
         with sentry_sdk.start_span(op="set_refs"):
             from sentry.api.exceptions import InvalidRepository
             from sentry.models import Commit, ReleaseHeadCommit, Repository
@@ -853,7 +853,7 @@ class Release(Model):
                 fetch_commits.apply_async(
                     kwargs={
                         "release_id": self.id,
-                        "user_id": user.id,
+                        "user_id": user_id,
                         "refs": refs,
                         "prev_release_id": prev_release and prev_release.id,
                     }

+ 1 - 1
src/sentry_plugins/heroku/plugin.py

@@ -138,7 +138,7 @@ class HerokuReleaseHook(ReleaseHook):
             else:
                 release.set_refs(
                     refs=[{"commit": release.version, "repository": repository.name}],
-                    user=values["owner"],
+                    user_id=values["owner"].id,
                     fetch=True,
                 )
         # create deploy associated with release via ReleaseDeploysEndpoint

+ 1 - 1
tests/acceptance/test_organization_releases.py

@@ -6,7 +6,7 @@ from sentry.testutils import AcceptanceTestCase
 from sentry.testutils.silo import region_silo_test
 
 
-@region_silo_test
+@region_silo_test(stable=True)
 class OrganizationReleasesTest(AcceptanceTestCase):
     release_date = datetime(2020, 5, 18, 15, 13, 58, 132928, tzinfo=timezone.utc)
 

+ 3 - 3
tests/sentry/api/endpoints/test_organization_releases.py

@@ -1136,7 +1136,7 @@ class OrganizationReleaseCreateTest(APITestCase):
         release = Release.objects.get(
             version=response.data["version"], user_agent="sentry-cli/2.77.4"
         )
-        assert not release.owner
+        assert not release.owner_id
         assert release.organization == org
         assert ReleaseProject.objects.filter(release=release, project=project).exists()
         assert ReleaseProject.objects.filter(release=release, project=project2).exists()
@@ -1338,7 +1338,7 @@ class OrganizationReleaseCreateTest(APITestCase):
         assert response.data["version"] == "1.2.3+dev"
 
         release = Release.objects.get(organization_id=org.id, version=response.data["version"])
-        assert not release.owner
+        assert not release.owner_id
 
     def test_features(self):
         user = self.create_user(is_staff=False, is_superuser=False)
@@ -1362,7 +1362,7 @@ class OrganizationReleaseCreateTest(APITestCase):
         assert response.data["version"]
 
         release = Release.objects.get(organization_id=org.id, version=response.data["version"])
-        assert release.owner == self.user
+        assert release.owner_id == self.user.id
 
     def test_commits(self):
         user = self.create_user(is_staff=False, is_superuser=False)

+ 9 - 8
tests/sentry/api/endpoints/test_project_releases.py

@@ -22,7 +22,7 @@ from sentry.testutils import APITestCase, ReleaseCommitPatchTest, TestCase
 from sentry.testutils.silo import region_silo_test
 
 
-@region_silo_test
+@region_silo_test(stable=True)
 class ProjectReleaseListTest(APITestCase):
     def test_simple(self):
         self.login_as(user=self.user)
@@ -114,7 +114,7 @@ class ProjectReleaseListTest(APITestCase):
         assert len(response.data) == 1
 
 
-@region_silo_test
+@region_silo_test(stable=True)
 class ProjectReleaseListEnvironmentsTest(APITestCase):
     def setUp(self):
         self.login_as(user=self.user)
@@ -316,7 +316,7 @@ class ProjectReleaseListEnvironmentsTest(APITestCase):
         )
 
 
-@region_silo_test
+@region_silo_test(stable=True)
 class ProjectReleaseCreateTest(APITestCase):
     def test_minimal(self):
         self.login_as(user=self.user)
@@ -340,7 +340,7 @@ class ProjectReleaseCreateTest(APITestCase):
             version=response.data["version"],
             user_agent="sentry-cli/2.77.4",
         )
-        assert not release.owner
+        assert not release.owner_id
         assert release.organization == project.organization
         assert release.projects.first() == project
 
@@ -359,7 +359,7 @@ class ProjectReleaseCreateTest(APITestCase):
         assert response.data["version"]
 
         release = Release.objects.get(version=response.data["version"])
-        assert not release.owner
+        assert not release.owner_id
         assert release.organization == project.organization
         assert release.projects.first() == project
 
@@ -438,7 +438,7 @@ class ProjectReleaseCreateTest(APITestCase):
         release = Release.objects.get(
             organization_id=project.organization_id, version=response.data["version"]
         )
-        assert not release.owner
+        assert not release.owner_id
 
     def test_features(self):
         self.login_as(user=self.user)
@@ -457,7 +457,7 @@ class ProjectReleaseCreateTest(APITestCase):
         release = Release.objects.get(
             organization_id=project.organization_id, version=response.data["version"]
         )
-        assert release.owner == self.user
+        assert release.owner_id == self.user.id
 
     def test_commits(self):
         self.login_as(user=self.user)
@@ -489,7 +489,7 @@ class ProjectReleaseCreateTest(APITestCase):
             assert rc.organization_id
 
 
-@region_silo_test
+@region_silo_test(stable=True)
 class ProjectReleaseCreateCommitPatch(ReleaseCommitPatchTest):
     @cached_property
     def url(self):
@@ -619,6 +619,7 @@ class ProjectReleaseCreateCommitPatch(ReleaseCommitPatchTest):
         }
 
 
+@region_silo_test(stable=True)
 class ReleaseSerializerTest(TestCase):
     def setUp(self):
         super().setUp()

+ 4 - 4
tests/sentry/models/test_release.py

@@ -607,7 +607,7 @@ class SetRefsTest(SetRefsTestCase):
             },
         ]
 
-        self.release.set_refs(refs, self.user, True)
+        self.release.set_refs(refs, self.user.id, True)
 
         commits = Commit.objects.all().order_by("id")
         self.assert_commit(commits[0], refs[0]["commit"])
@@ -635,7 +635,7 @@ class SetRefsTest(SetRefsTestCase):
         ]
 
         with pytest.raises(InvalidRepository):
-            self.release.set_refs(refs, self.user)
+            self.release.set_refs(refs, self.user.id)
 
         assert len(Commit.objects.all()) == 0
         assert len(ReleaseHeadCommit.objects.all()) == 0
@@ -657,7 +657,7 @@ class SetRefsTest(SetRefsTestCase):
             {"repository": "test/repo", "commit": "previous-commit-id-3..current-commit-id-3"},
         ]
 
-        self.release.set_refs(refs, self.user, True)
+        self.release.set_refs(refs, self.user.id, True)
 
         commits = Commit.objects.all().order_by("id")
         self.assert_commit(commits[0], "current-commit-id")
@@ -685,7 +685,7 @@ class SetRefsTest(SetRefsTestCase):
             },
         ]
 
-        self.release.set_refs(refs, self.user, False)
+        self.release.set_refs(refs, self.user.id, False)
 
         commits = Commit.objects.all().order_by("id")
         self.assert_commit(commits[0], refs[0]["commit"])

+ 22 - 15
tests/sentry/receivers/test_releases.py

@@ -26,11 +26,11 @@ from sentry.models import (
 )
 from sentry.signals import buffer_incr_complete, receivers_raise_on_send
 from sentry.testutils import TestCase
-from sentry.testutils.silo import region_silo_test
+from sentry.testutils.silo import exempt_from_silo_limits, region_silo_test
 from sentry.types.activity import ActivityType
 
 
-@region_silo_test
+@region_silo_test(stable=True)
 class ResolveGroupResolutionsTest(TestCase):
     @patch("sentry.tasks.clear_expired_resolutions.clear_expired_resolutions.delay")
     def test_simple(self, mock_delay):
@@ -43,6 +43,7 @@ class ResolveGroupResolutionsTest(TestCase):
         mock_delay.assert_called_once_with(release_id=release.id)
 
 
+@region_silo_test(stable=True)
 class ResolvedInCommitTest(TestCase):
     def assertResolvedFromCommit(self, group, commit):
         assert GroupLink.objects.filter(
@@ -164,12 +165,15 @@ class ResolvedInCommitTest(TestCase):
         group = self.create_group()
         add_group_to_inbox(group, GroupInboxReason.MANUAL)
         user = self.create_user(name="Foo Bar", email="foo@example.com", is_active=True)
-        email = UserEmail.objects.get_primary_email(user=user)
+        with exempt_from_silo_limits():
+            email = UserEmail.objects.get_primary_email(user=user)
         email.is_verified = True
-        email.save()
+        with exempt_from_silo_limits():
+            email.save()
         repo = Repository.objects.create(name="example", organization_id=self.group.organization.id)
         OrganizationMember.objects.create(organization=group.project.organization, user=user)
-        UserOption.objects.set_value(user=user, key="self_assign_issue", value="1")
+        with exempt_from_silo_limits():
+            UserOption.objects.set_value(user=user, key="self_assign_issue", value="1")
 
         commit = Commit.objects.create(
             key=sha1(uuid4().hex.encode("utf-8")).hexdigest(),
@@ -183,7 +187,7 @@ class ResolvedInCommitTest(TestCase):
 
         self.assertResolvedFromCommit(group, commit)
 
-        assert GroupAssignee.objects.filter(group=group, user=user).exists()
+        assert GroupAssignee.objects.filter(group=group, user_id=user.id).exists()
 
         assert Activity.objects.filter(
             project=group.project, group=group, type=ActivityType.ASSIGNED.value, user=user
@@ -193,19 +197,22 @@ class ResolvedInCommitTest(TestCase):
             "assigneeType": "user",
         }
 
-        assert GroupSubscription.objects.filter(group=group, user=user).exists()
+        assert GroupSubscription.objects.filter(group=group, user_id=user.id).exists()
 
     @receivers_raise_on_send()
     def test_matching_author_without_assignment(self):
         group = self.create_group()
         add_group_to_inbox(group, GroupInboxReason.MANUAL)
         user = self.create_user(name="Foo Bar", email="foo@example.com", is_active=True)
-        email = UserEmail.objects.get_primary_email(user=user)
-        email.is_verified = True
-        email.save()
-        repo = Repository.objects.create(name="example", organization_id=self.group.organization.id)
-        OrganizationMember.objects.create(organization=group.project.organization, user=user)
-        UserOption.objects.set_value(user=user, key="self_assign_issue", value="0")
+        with exempt_from_silo_limits():
+            email = UserEmail.objects.get_primary_email(user=user)
+            email.is_verified = True
+            email.save()
+            repo = Repository.objects.create(
+                name="example", organization_id=self.group.organization.id
+            )
+            OrganizationMember.objects.create(organization=group.project.organization, user=user)
+            UserOption.objects.set_value(user=user, key="self_assign_issue", value="0")
 
         commit = Commit.objects.create(
             key=sha1(uuid4().hex.encode("utf-8")).hexdigest(),
@@ -223,10 +230,10 @@ class ResolvedInCommitTest(TestCase):
             project=group.project, group=group, type=ActivityType.ASSIGNED.value, user=user
         ).exists()
 
-        assert GroupSubscription.objects.filter(group=group, user=user).exists()
+        assert GroupSubscription.objects.filter(group=group, user_id=user.id).exists()
 
 
-@region_silo_test
+@region_silo_test(stable=True)
 class ProjectHasReleasesReceiverTest(TestCase):
     @receivers_raise_on_send()
     def test(self):