chore(ACI): Remove support for legacy sentry app data in Actions#107174
chore(ACI): Remove support for legacy sentry app data in Actions#107174
Conversation
github-actions
bot
added
the
Scope: Backend
| @mock.patch( | ||
| "sentry.rules.actions.sentry_apps.utils.app_service.trigger_sentry_app_action_creators" | ||
| ) | ||
| def test_validate_sentry_app_id( |
There was a problem hiding this comment.
The basic test_validate covers this case now that the setup data is using sentry app id
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| installation = self._get_sentry_app_installation(sentry_app_identifier, target_identifier) | ||
| installations = app_service.get_many( | ||
| filter=dict(app_ids=[int(target_identifier)], organization_id=self.organization.id) | ||
| ) |
There was a problem hiding this comment.
Unhandled ValueError when legacy identifier format used
Medium Severity
The clean_data method calls int(target_identifier) without error handling. The config schema in sentry_app_handler.py still includes SENTRY_APP_INSTALLATION_UUID in the sentry_app_identifier enum, allowing requests with UUID-formatted target_identifier values to pass schema validation. When such a request reaches this code, int() raises an unhandled ValueError, resulting in a 500 error instead of a proper validation error response.
There was a problem hiding this comment.
that is why we don't merge this until the migration has run
ceorourke
force-pushed
the
ceorourke/remove-support-legacy-action-sentry-app-data
branch
from
41f67a9 to
71eac0d
Compare
ceorourke
force-pushed
the
ceorourke/remove-support-legacy-action-sentry-app-data
branch
from
71eac0d to
c649a47
Compare
ceorourke
force-pushed
the
ceorourke/remove-support-legacy-action-sentry-app-data
branch
from
fc008f5 to
823bc8b
Compare
1 participant
Remove the code that handles both the legacy sentry app installation uuid and the sentry app id as the target identifier. Now that we've migrated the
Actiondata to only ever have the sentry app id, we don't need to expect the installation uuid.Do not merge until we've verified the migration ran successfully.
As a follow up I don't believe we're using
Action.config["sentry_app_identifier"]anymore (since it's always sentry app id) so it might be nice to run a final cleanup migration to remove the field.