Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(sentryapps) Remove uncached RPC method #81153

Merged
merged 7 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/sentry/sentry_apps/services/app/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
16 changes: 1 addition & 15 deletions src/sentry/sentry_apps/services/app/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]:
Expand All @@ -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
Expand Down
49 changes: 0 additions & 49 deletions tests/sentry/sentry_apps/tasks/test_sentry_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
Loading