Skip to content

chore(sentry-apps): move sentry apps to async deletion #100840

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 3, 2025
Merged

chore(sentry-apps): move sentry apps to async deletion #100840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 4 additions & 2 deletions src/sentry/models/authprovider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
from sentry.backup.dependencies import NormalizedModelName, get_model_name
from sentry.backup.sanitize import SanitizableField, Sanitizer
from sentry.backup.scopes import RelocationScope
from sentry.constants import SentryAppStatus
from sentry.db.models import BoundedPositiveIntegerField, control_silo_model, sane_repr
from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
from sentry.deletions.models.scheduleddeletion import ScheduledDeletion
from sentry.hybridcloud.models.outbox import ControlOutbox
from sentry.hybridcloud.outbox.base import ReplicatedControlModel
from sentry.hybridcloud.outbox.category import OutboxCategory, OutboxScope
Expand Down Expand Up @@ -164,7 +166,6 @@ def outboxes_for_reset_idp_flags(self) -> list[ControlOutbox]:
]

def disable_scim(self):
from sentry import deletions
from sentry.sentry_apps.models.sentry_app_installation_for_provider import (
SentryAppInstallationForProvider,
)
Expand All @@ -185,7 +186,8 @@ def disable_scim(self):
assert (
sentry_app.is_internal
), "scim sentry apps should always be internal, thus deleting them without triggering InstallationNotifier is correct."
deletions.exec_sync(sentry_app)
sentry_app.update(status=SentryAppStatus.DELETION_IN_PROGRESS)
ScheduledDeletion.schedule(sentry_app, days=0)
except SentryAppInstallationForProvider.DoesNotExist:
pass
self.flags.scim_enabled = False
Expand Down
5 changes: 4 additions & 1 deletion src/sentry/sentry_apps/api/endpoints/sentry_app_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from sentry.apidocs.utils import inline_sentry_response_serializer
from sentry.auth.staff import is_active_staff
from sentry.constants import SentryAppStatus
from sentry.deletions.models.scheduleddeletion import ScheduledDeletion
from sentry.organizations.services.organization import organization_service
from sentry.sentry_apps.api.bases.sentryapps import (
SentryAppAndStaffPermission,
Expand Down Expand Up @@ -234,13 +235,15 @@ def delete(self, request: Request, sentry_app) -> Response:
sentry_sdk.capture_exception(exc)

with transaction.atomic(using=router.db_for_write(SentryApp)):
deletions.exec_sync(sentry_app)
sentry_app.update(status=SentryAppStatus.DELETION_IN_PROGRESS)
scheduled = ScheduledDeletion.schedule(sentry_app, days=0, actor=request.user)
Comment on lines +238 to +239
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we find ourselves using this pattern in other contexts, let's consider moving this to a service class that manages sentry app deletions/updates for us. Encoding this behavior in endpoints has bit us in the past when implementations differ by mistake.

create_audit_entry(
request=request,
organization_id=sentry_app.owner_id,
target_object=sentry_app.owner_id,
event=audit_log.get_event_id("SENTRY_APP_REMOVE"),
data={"sentry_app": sentry_app.name},
transaction_id=scheduled.id,
)
if request.user.is_authenticated:
analytics.record(
Expand Down
5 changes: 5 additions & 0 deletions tests/sentry/sentry_apps/api/endpoints/test_sentry_app_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
SentryAppSchemaValidationError,
)
from sentry.constants import SentryAppStatus
from sentry.deletions.tasks.scheduled import run_scheduled_deletions_control
from sentry.models.auditlogentry import AuditLogEntry
from sentry.models.organizationmember import OrganizationMember
from sentry.sentry_apps.api.endpoints.sentry_app_details import PARTNERSHIP_RESTRICTED_ERROR_MESSAGE
Expand Down Expand Up @@ -791,6 +792,8 @@ def test_superuser_delete_unpublished_app(self, record: MagicMock) -> None:
self.unpublished_app.slug,
status_code=204,
)
with self.tasks():
run_scheduled_deletions_control()

assert AuditLogEntry.objects.filter(
event=audit_log.get_event_id("SENTRY_APP_REMOVE")
Expand All @@ -815,6 +818,8 @@ def test_superuser_delete_unpublished_app_with_installs(self) -> None:
self.unpublished_app.slug,
status_code=204,
)
with self.tasks():
run_scheduled_deletions_control()

assert AuditLogEntry.objects.filter(
event=audit_log.get_event_id("SENTRY_APP_REMOVE")
Expand Down
4 changes: 4 additions & 0 deletions tests/sentry/web/frontend/test_organization_auth_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
)
from sentry.auth.providers.saml2.generic.provider import GenericSAML2Provider
from sentry.auth.providers.saml2.provider import Attributes
from sentry.deletions.tasks.scheduled import run_scheduled_deletions_control
from sentry.models.auditlogentry import AuditLogEntry
from sentry.models.authidentity import AuthIdentity
from sentry.models.authprovider import AuthProvider
Expand Down Expand Up @@ -604,6 +605,9 @@ def test_edit_sso_settings__scim(self) -> None:
},
)

with self.tasks():
run_scheduled_deletions_control()

assert resp.status_code == 200
auth_provider = AuthProvider.objects.get(organization_id=organization.id)

Expand Down
Loading