-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
chore(grouphistory): Stop using prev_history
#102474
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
Conversation
While investigating #102470, I realized we don't actually use it for anything useful. In order to delete the column, we first need to stop making any reference to it ([docs](https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can go straight into removing the usage of prev_history since it's already null.
First, if the column is either not nullable, or doesn't have a db_default set, then make a PR to make it nullable via null=True.
sentry/src/sentry/models/grouphistory.py
Lines 215 to 217 in 5871107
| prev_history = FlexibleForeignKey( | |
| "sentry.GroupHistory", null=True | |
| ) # This field has no immediate use, but might be useful. |
| user_id: int | None = None, | ||
| team_id: int | None = None, | ||
| prev_history: GroupHistory | None = None, | ||
| prev_history_date: datetime | None = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, this is the column which can be null or a datetime:
sentry/src/sentry/models/grouphistory.py
Lines 218 to 220 in 5871107
| prev_history_date = models.DateTimeField( | |
| null=True | |
| ) # This field is used to simplify query calculations. |
| GroupHistoryStatus.RESOLVED, | ||
| user_id=self.user.id, | ||
| prev_history=gh1, | ||
| prev_history_date=gh1.date_added, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, the date_added field:
sentry/src/sentry/models/grouphistory.py
Line 221 in 5871107
| date_added = models.DateTimeField(default=timezone.now) |
cvxluo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we planning to remove prev_history but keep prev_history_date?
That's correct! |
While investigating #102470, I realized we don't actually use it for anything useful. In order to delete the column, we first need to stop making use of it (see [docs](https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns) for instructions).
This is part of the required steps to remove a column from Sentry: https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns. This is a follow-up to #102474. This fixes [SENTRY-5BX8](https://sentry.sentry.io/issues/6994851757/) and [SENTRY-5BXW](https://sentry.sentry.io/issues/6995699817/) which are happening since we switched to the bulk deletion model (#102580).
This is part of the required steps to remove a column from Sentry: https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns. This is a follow-up to #102474. This fixes [SENTRY-5BX8](https://sentry.sentry.io/issues/6994851757/) and [SENTRY-5BXW](https://sentry.sentry.io/issues/6995699817/) which are happening since we switched to the bulk deletion model (#102580).
While investigating #102470, I realized we don't actually use it for anything useful. In order to delete the column, we first need to stop making use of it (see [docs](https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns) for instructions).
This is part of the required steps to remove a column from Sentry: https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns. This is a follow-up to #102474. This fixes [SENTRY-5BX8](https://sentry.sentry.io/issues/6994851757/) and [SENTRY-5BXW](https://sentry.sentry.io/issues/6995699817/) which are happening since we switched to the bulk deletion model (#102580).
This is part of the required steps to remove a column from Sentry: https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-columns. This is a follow-up to #102474. This fixes [SENTRY-5BX8](https://sentry.sentry.io/issues/6994851757/) and [SENTRY-5BXW](https://sentry.sentry.io/issues/6995699817/) which are happening since we switched to the bulk deletion model (#102580).
While investigating #102470, I realized we don't actually use it for anything useful.
In order to delete the column, we first need to stop making use of it (see docs for instructions).
Next PR: #102672