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

refs(incidents): Remove unused incident stat snapshots and endpoint #29897

Merged
merged 1 commit into from
Nov 10, 2021

Conversation

wedamija
Copy link
Member

We stopped using the incident stats endpoint a while ago when we refactored the alerts page. Since
this endpoint is no longer used, it also means that we no longer need to keep historical snapshots
of incident statistics. Removing this whole system since it's complicated and a pain to work with.

We stopped using the incident stats endpoint a while ago when we refactored the alerts page. Since
this endpoint is no longer used, it also means that we no longer need to keep historical snapshots
of incident statistics. Removing this whole system since it's complicated and a pain to work with.
@wedamija wedamija requested review from a team November 10, 2021 00:21
@wedamija wedamija requested a review from a team as a code owner November 10, 2021 00:21
@github-actions
Copy link
Contributor

This PR has a migration; here is the generated SQL

BEGIN;
--
-- Alter field event_stats_snapshot on incidentsnapshot
--
SET CONSTRAINTS "sentry_incidentsnaps_event_stats_snapshot_9bdbecc5_fk_sentry_ti" IMMEDIATE; ALTER TABLE "sentry_incidentsnapshot" DROP CONSTRAINT "sentry_incidentsnaps_event_stats_snapshot_9bdbecc5_fk_sentry_ti";
--
-- Alter field incident on incidentsnapshot
--
SET CONSTRAINTS "sentry_incidentsnaps_incident_id_855af8a4_fk_sentry_in" IMMEDIATE; ALTER TABLE "sentry_incidentsnapshot" DROP CONSTRAINT "sentry_incidentsnaps_incident_id_855af8a4_fk_sentry_in";
--
-- Alter field incident on pendingincidentsnapshot
--
SET CONSTRAINTS "sentry_pendingincide_incident_id_c02c4af7_fk_sentry_in" IMMEDIATE; ALTER TABLE "sentry_pendingincidentsnapshot" DROP CONSTRAINT "sentry_pendingincide_incident_id_c02c4af7_fk_sentry_in";
COMMIT;

Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Migrations look good to me.

@instrumented_task(
name="sentry.incidents.tasks.process_pending_incident_snapshots", queue="incident_snapshots"
)
def process_pending_incident_snapshots(next_id=None):
Copy link
Member

Choose a reason for hiding this comment

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

Could we have messages in the queues for this task during a deploy? Should this method stay but as a no-op, or do we ignore any sentry errors that may come from this task vanishing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I think I'm ok with a couple of errors firing for this task, shouldn't cause any actual problems.

@wedamija wedamija merged commit 5f861f4 into master Nov 10, 2021
@wedamija wedamija deleted the danf/remove_incident_snapshots branch November 10, 2021 18:42
@github-actions github-actions bot locked and limited conversation to collaborators Nov 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants