-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
ref(crons): Drop last_state_change from db #66429
ref(crons): Drop last_state_change from db #66429
Conversation
This PR has a migration; here is the generated SQL for --
-- Custom state/database change combination
--
ALTER TABLE "sentry_monitorenvironment" DROP COLUMN "last_state_change"; |
ALTER TABLE "sentry_monitorenvironment" DROP COLUMN "last_state_change"; | ||
""", | ||
reverse_sql=""" | ||
ALTER TABLE "sentry_monitorenvironment" ADD COLUMN "last_state_change" timestamptz; |
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.
Add it as nullable here. Mostly for dev but this reverse will fail otherwise
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.
I think default it is nullable without NOT NULL
right?
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.
In the current implementation of ADD COLUMN, default and NOT NULL clauses for the new column are not supported. The new column always comes into being with all values NULL. You can use the SET DEFAULT form of ALTER TABLE to set the default afterwards.
right?
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.
I always get this backwards, because in Django it's the opposite!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #66429 +/- ##
===========================================
+ Coverage 57.49% 84.29% +26.80%
===========================================
Files 5292 5309 +17
Lines 236552 237233 +681
Branches 40959 41042 +83
===========================================
+ Hits 135999 199974 +63975
+ Misses 100329 37040 -63289
+ Partials 224 219 -5 |
fdad04d
to
741f92a
Compare
This PR has a migration; here is the generated SQL for --
-- Custom state/database change combination
--
ALTER TABLE "sentry_monitorenvironment" DROP COLUMN "last_state_change"; |
No longer needed after GH-66353
No longer needed after GH-66353