From 2889757821ae98e0349fa695893737b8fd932394 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 21 Nov 2024 15:56:48 -0500 Subject: [PATCH 1/3] chore(sentryapps) Remove uncached RPC method With all traffic using the cached method call we can remove the deprecated RPC call. Refs #80488 --- src/sentry/sentry_apps/services/app/impl.py | 6 --- .../sentry_apps/services/app/service.py | 16 +----- .../sentry_apps/tasks/test_sentry_apps.py | 49 ------------------- 3 files changed, 1 insertion(+), 70 deletions(-) diff --git a/src/sentry/sentry_apps/services/app/impl.py b/src/sentry/sentry_apps/services/app/impl.py index 218d010f8436a..e266283516b35 100644 --- a/src/sentry/sentry_apps/services/app/impl.py +++ b/src/sentry/sentry_apps/services/app/impl.py @@ -97,12 +97,6 @@ def get_sentry_app_by_slug(self, *, slug: str) -> RpcSentryApp | None: except SentryApp.DoesNotExist: return None - def get_installed_for_organization( - self, *, organization_id: int - ) -> list[RpcSentryAppInstallation]: - # Deprecated. Use get_installations_for_organization instead. - return self.get_installations_for_organization(organization_id=organization_id) - def get_installations_for_organization( self, *, organization_id: int ) -> list[RpcSentryAppInstallation]: diff --git a/src/sentry/sentry_apps/services/app/service.py b/src/sentry/sentry_apps/services/app/service.py index 731c2f6573fbc..7d8d8f466fcc6 100644 --- a/src/sentry/sentry_apps/services/app/service.py +++ b/src/sentry/sentry_apps/services/app/service.py @@ -8,7 +8,6 @@ from typing import Any from sentry.auth.services.auth import AuthenticationContext -from sentry.features.rollout import in_random_rollout from sentry.hybridcloud.rpc.caching.service import back_with_silo_cache, back_with_silo_cache_list from sentry.hybridcloud.rpc.filter_query import OpaqueSerializedResponse from sentry.hybridcloud.rpc.service import RpcService, rpc_method @@ -61,16 +60,6 @@ def find_installation_by_proxy_user( ) -> RpcSentryAppInstallation | None: pass - @rpc_method - @abc.abstractmethod - def get_installed_for_organization( - self, - *, - organization_id: int, - ) -> list[RpcSentryAppInstallation]: - # Deprecated use installations_for_organization instead. - pass - def installations_for_organization( self, *, organization_id: int ) -> list[RpcSentryAppInstallation]: @@ -79,10 +68,7 @@ def installations_for_organization( This is a cached wrapper around get_installations_for_organization """ - if in_random_rollout("app_service.installations_for_org.cached"): - return get_installations_for_organization(organization_id) - else: - return self.get_installed_for_organization(organization_id=organization_id) + return get_installations_for_organization(organization_id) @rpc_method @abc.abstractmethod diff --git a/tests/sentry/sentry_apps/tasks/test_sentry_apps.py b/tests/sentry/sentry_apps/tasks/test_sentry_apps.py index cf3a15b61ab3a..e289f49e55127 100644 --- a/tests/sentry/sentry_apps/tasks/test_sentry_apps.py +++ b/tests/sentry/sentry_apps/tasks/test_sentry_apps.py @@ -37,7 +37,6 @@ from sentry.testutils.helpers import with_feature from sentry.testutils.helpers.datetime import before_now, freeze_time from sentry.testutils.helpers.eventprocessing import write_event_to_cache -from sentry.testutils.helpers.options import override_options from sentry.testutils.outbox import outbox_runner from sentry.testutils.silo import assume_test_silo_mode_of, control_silo_test from sentry.testutils.skips import requires_snuba @@ -333,54 +332,6 @@ def test_error_created_sends_webhook(self, safe_urlopen): "Sentry-Hook-Signature", } - @with_feature("organizations:integrations-event-hooks") - @override_options({"app_service.installations_for_org.cached": 1.0}) - def test_error_created_sends_webhook_cached_service_call(self, safe_urlopen): - sentry_app = self.create_sentry_app( - organization=self.project.organization, events=["error.created"] - ) - install = self.create_sentry_app_installation( - organization=self.project.organization, slug=sentry_app.slug - ) - - one_min_ago = before_now(minutes=1).isoformat() - event = self.store_event( - data={ - "message": "Foo bar", - "exception": {"type": "Foo", "value": "oh no"}, - "level": "error", - "timestamp": one_min_ago, - }, - project_id=self.project.id, - assert_no_errors=False, - ) - - with self.tasks(): - post_process_group( - is_new=False, - is_regression=False, - is_new_group_environment=False, - cache_key=write_event_to_cache(event), - group_id=event.group_id, - project_id=self.project.id, - eventstream_type=EventStreamEventType.Error, - ) - - ((args, kwargs),) = safe_urlopen.call_args_list - data = json.loads(kwargs["data"]) - - assert data["action"] == "created" - assert data["installation"]["uuid"] == install.uuid - assert data["data"]["error"]["event_id"] == event.event_id - assert data["data"]["error"]["issue_id"] == str(event.group_id) - assert kwargs["headers"].keys() >= { - "Content-Type", - "Request-ID", - "Sentry-Hook-Resource", - "Sentry-Hook-Timestamp", - "Sentry-Hook-Signature", - } - # TODO(nola): Enable this test whenever we prevent infinite loops w/ error.created integrations @pytest.mark.skip(reason="enable this when/if we do prevent infinite error.created loops") @with_feature("organizations:integrations-event-hooks") From 5b834e1ecb0df4738c21201d3531f884e05360e6 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 22 Nov 2024 13:03:51 -0500 Subject: [PATCH 2/3] Restore old method to try and appease CI --- src/sentry/sentry_apps/services/app/impl.py | 6 ++++++ src/sentry/sentry_apps/services/app/service.py | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/sentry/sentry_apps/services/app/impl.py b/src/sentry/sentry_apps/services/app/impl.py index e266283516b35..218d010f8436a 100644 --- a/src/sentry/sentry_apps/services/app/impl.py +++ b/src/sentry/sentry_apps/services/app/impl.py @@ -97,6 +97,12 @@ def get_sentry_app_by_slug(self, *, slug: str) -> RpcSentryApp | None: except SentryApp.DoesNotExist: return None + def get_installed_for_organization( + self, *, organization_id: int + ) -> list[RpcSentryAppInstallation]: + # Deprecated. Use get_installations_for_organization instead. + return self.get_installations_for_organization(organization_id=organization_id) + def get_installations_for_organization( self, *, organization_id: int ) -> list[RpcSentryAppInstallation]: diff --git a/src/sentry/sentry_apps/services/app/service.py b/src/sentry/sentry_apps/services/app/service.py index 7d8d8f466fcc6..7e6545b5977b3 100644 --- a/src/sentry/sentry_apps/services/app/service.py +++ b/src/sentry/sentry_apps/services/app/service.py @@ -60,6 +60,16 @@ def find_installation_by_proxy_user( ) -> RpcSentryAppInstallation | None: pass + @rpc_method + @abc.abstractmethod + def get_installed_for_organization( + self, + *, + organization_id: int, + ) -> list[RpcSentryAppInstallation]: + # Deprecated use installations_for_organization instead. + pass + def installations_for_organization( self, *, organization_id: int ) -> list[RpcSentryAppInstallation]: From 8a97af1738808c6c6622fa66b9df57d23cd3560e Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 22 Nov 2024 13:36:44 -0500 Subject: [PATCH 3/3] Remove the methods --- src/sentry/sentry_apps/services/app/impl.py | 6 ------ src/sentry/sentry_apps/services/app/service.py | 10 ---------- 2 files changed, 16 deletions(-) diff --git a/src/sentry/sentry_apps/services/app/impl.py b/src/sentry/sentry_apps/services/app/impl.py index 218d010f8436a..e266283516b35 100644 --- a/src/sentry/sentry_apps/services/app/impl.py +++ b/src/sentry/sentry_apps/services/app/impl.py @@ -97,12 +97,6 @@ def get_sentry_app_by_slug(self, *, slug: str) -> RpcSentryApp | None: except SentryApp.DoesNotExist: return None - def get_installed_for_organization( - self, *, organization_id: int - ) -> list[RpcSentryAppInstallation]: - # Deprecated. Use get_installations_for_organization instead. - return self.get_installations_for_organization(organization_id=organization_id) - def get_installations_for_organization( self, *, organization_id: int ) -> list[RpcSentryAppInstallation]: diff --git a/src/sentry/sentry_apps/services/app/service.py b/src/sentry/sentry_apps/services/app/service.py index 7e6545b5977b3..7d8d8f466fcc6 100644 --- a/src/sentry/sentry_apps/services/app/service.py +++ b/src/sentry/sentry_apps/services/app/service.py @@ -60,16 +60,6 @@ def find_installation_by_proxy_user( ) -> RpcSentryAppInstallation | None: pass - @rpc_method - @abc.abstractmethod - def get_installed_for_organization( - self, - *, - organization_id: int, - ) -> list[RpcSentryAppInstallation]: - # Deprecated use installations_for_organization instead. - pass - def installations_for_organization( self, *, organization_id: int ) -> list[RpcSentryAppInstallation]: