-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
fix(aci): Change open period range boundary to inclusive #93535
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
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #93535 +/- ##
========================================
Coverage 88.03% 88.04%
========================================
Files 10325 10330 +5
Lines 595677 596288 +611
Branches 23134 23134
========================================
+ Hits 524411 524989 +578
- Misses 70773 70806 +33
Partials 493 493 |
Hm |
Talked to @wedamija - we'll turn off the ff for writes and run the post-deploy migration to delete rows before running this migration with SafeRunSQL. |
This PR has a migration; here is the generated SQL for for --
-- Custom state/database change combination
--
ALTER TABLE "sentry_groupopenperiod" ADD CONSTRAINT "exclude_overlapping_date_start_end" EXCLUDE USING GIST ("group" WITH =, (TSTZRANGE("date_started", "date_ended", '[]')) WITH &&);
--
-- Remove constraint exclude_overlapping_start_end from model groupopenperiod
--
ALTER TABLE "sentry_groupopenperiod" DROP CONSTRAINT "exclude_overlapping_start_end"; |
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.
Btw, if you want to delete the table quickly, you can use a TRUNCATE
on the table so that you don't have to wait.
This PR has a migration; here is the generated SQL for for --
-- Custom state/database change combination
--
ALTER TABLE "sentry_groupopenperiod" ADD CONSTRAINT "exclude_overlapping_date_start_end" EXCLUDE USING GIST ("group_id" WITH =, (TSTZRANGE("date_started", "date_ended", '[]')) WITH &&);
--
-- Remove constraint exclude_overlapping_start_end from model groupopenperiod
--
ALTER TABLE "sentry_groupopenperiod" DROP CONSTRAINT "exclude_overlapping_start_end"; |
src/sentry/models/groupopenperiod.py
Outdated
expressions=[ | ||
(models.F("group"), RangeOperators.EQUAL), | ||
("group", "="), |
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.
Why do we change this to be the string? Can't you just keep it as modes.F(...)
?
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.
hm that was something I had changed when the migration check kept failing, but it shouldn't be necessary
Some issues have the same regression/resolution time in our db and the range boundary today would reject those as valid start/end times for open periods. Updating the boundary to be inclusive on both ends should fix this and allow an open period to start at the same time that the previous one ends.
Some issues have the same regression/resolution time in our db and the range boundary today would reject those as valid start/end times for open periods. Updating the boundary to be inclusive on both ends should fix this and allow an open period to start at the same time that the previous one ends.