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

feat(team-workflow): Add team to GroupSubscription #55805

Merged
merged 8 commits into from
Sep 7, 2023

Conversation

isabellaenriquez
Copy link
Member

@isabellaenriquez isabellaenriquez commented Sep 6, 2023

Adds team to the GroupSubscription table and adds constraint that at least one of team_id or user_id is set.

Close #55555.

@isabellaenriquez isabellaenriquez requested a review from a team as a code owner September 6, 2023 20:14
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 6, 2023
@isabellaenriquez isabellaenriquez self-assigned this Sep 6, 2023
@isabellaenriquez isabellaenriquez requested a review from a team September 6, 2023 20:16
@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

This PR has a migration; here is the generated SQL for src/sentry/migrations/0544_add_team_id_to_groupsubscription.py ()

--
-- Add field team to groupsubscription
--
ALTER TABLE "sentry_groupsubscription" ADD COLUMN "team_id" bigint NULL;
--
-- Alter field user_id on groupsubscription
--
ALTER TABLE "sentry_groupsubscription" ALTER COLUMN "user_id" DROP NOT NULL;
--
-- Alter unique_together for groupsubscription (2 constraint(s))
--
CREATE UNIQUE INDEX CONCURRENTLY "sentry_groupsubscription_group_id_team_id_ba7c937c_uniq" ON "sentry_groupsubscription" ("group_id", "team_id");
ALTER TABLE "sentry_groupsubscription" ADD CONSTRAINT "sentry_groupsubscription_group_id_team_id_ba7c937c_uniq" UNIQUE USING INDEX "sentry_groupsubscription_group_id_team_id_ba7c937c_uniq";
ALTER TABLE "sentry_groupsubscription" ADD CONSTRAINT "sentry_groupsubscription_team_id_255af5da_fk_sentry_team_id" FOREIGN KEY ("team_id") REFERENCES "sentry_team" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_groupsubscription" VALIDATE CONSTRAINT "sentry_groupsubscription_team_id_255af5da_fk_sentry_team_id";
CREATE INDEX CONCURRENTLY "sentry_groupsubscription_team_id_255af5da" ON "sentry_groupsubscription" ("team_id");
--
-- Create constraint subscription_team_or_user_check on model groupsubscription
--
ALTER TABLE "sentry_groupsubscription" ADD CONSTRAINT "subscription_team_or_user_check" CHECK ((("team_id" IS NOT NULL AND "user_id" IS NULL) OR ("team_id" IS NULL AND "user_id" IS NOT NULL))) NOT VALID;
ALTER TABLE "sentry_groupsubscription" VALIDATE CONSTRAINT "subscription_team_or_user_check";

@@ -178,7 +178,8 @@ class GroupSubscription(Model):

project = FlexibleForeignKey("sentry.Project", related_name="subscription_set")
group = FlexibleForeignKey("sentry.Group", related_name="subscription_set")
user_id = HybridCloudForeignKey(settings.AUTH_USER_MODEL, on_delete="CASCADE")
user_id = HybridCloudForeignKey(settings.AUTH_USER_MODEL, null=True, on_delete="CASCADE")
team_id = HybridCloudForeignKey("sentry.Team", null=True, db_index=True, on_delete="CASCADE")
Copy link
Contributor

Choose a reason for hiding this comment

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

@isabellaenriquez since this is in the region silo, I think we can just use FlexibleForeignKey instead of HybridCloudForeignKey

Copy link
Member

Choose a reason for hiding this comment

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

Yes, Steve is correct, HybridCloudForeignKey should only be used when a relationship spans regions. You want FlexibleForeignKey here.

@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Merging #55805 (965fd43) into master (b00bfce) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #55805   +/-   ##
=======================================
  Coverage   79.96%   79.97%           
=======================================
  Files        5055     5055           
  Lines      217220   217222    +2     
  Branches    36784    36784           
=======================================
+ Hits       173705   173714    +9     
+ Misses      38186    38180    -6     
+ Partials     5329     5328    -1     
Files Changed Coverage
src/sentry/models/groupsubscription.py 100.00%

src/sentry/models/groupsubscription.py Outdated Show resolved Hide resolved
src/sentry/models/groupsubscription.py Outdated Show resolved Hide resolved
# - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
# have ops run this and not block the deploy. Note that while adding an index is a schema
# change, it's completely safe to run the operation after the code has deployed.
is_dangerous = False
Copy link
Member

Choose a reason for hiding this comment

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

The groupsubscription table has ~30M rows. You should fit within the deployment timeout, but if you want to be cautious you should make this migration is_dangerous = True and have SRE run it.

@isabellaenriquez isabellaenriquez changed the title feat(team-workflow): Add team ID to GroupSubscription feat(team-workflow): Add team to GroupSubscription Sep 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

This PR has a migration; here is the generated SQL for src/sentry/migrations/0543_add_team_id_to_groupsubscription.py ()

--
-- Add field team to groupsubscription
--
ALTER TABLE "sentry_groupsubscription" ADD COLUMN "team_id" bigint NULL;
--
-- Alter field user_id on groupsubscription
--
ALTER TABLE "sentry_groupsubscription" ALTER COLUMN "user_id" DROP NOT NULL;
--
-- Alter unique_together for groupsubscription (2 constraint(s))
--
CREATE UNIQUE INDEX CONCURRENTLY "sentry_groupsubscription_group_id_team_id_ba7c937c_uniq" ON "sentry_groupsubscription" ("group_id", "team_id");
ALTER TABLE "sentry_groupsubscription" ADD CONSTRAINT "sentry_groupsubscription_group_id_team_id_ba7c937c_uniq" UNIQUE USING INDEX "sentry_groupsubscription_group_id_team_id_ba7c937c_uniq";
ALTER TABLE "sentry_groupsubscription" ADD CONSTRAINT "sentry_groupsubscription_team_id_255af5da_fk_sentry_team_id" FOREIGN KEY ("team_id") REFERENCES "sentry_team" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_groupsubscription" VALIDATE CONSTRAINT "sentry_groupsubscription_team_id_255af5da_fk_sentry_team_id";
CREATE INDEX CONCURRENTLY "sentry_groupsubscription_team_id_255af5da" ON "sentry_groupsubscription" ("team_id");
--
-- Create constraint subscription_team_or_user_check on model groupsubscription
--
ALTER TABLE "sentry_groupsubscription" ADD CONSTRAINT "subscription_team_or_user_check" CHECK ((("team_id" IS NOT NULL AND "user_id" IS NULL) OR ("team_id" IS NULL AND "user_id" IS NOT NULL))) NOT VALID;
ALTER TABLE "sentry_groupsubscription" VALIDATE CONSTRAINT "subscription_team_or_user_check";

@isabellaenriquez isabellaenriquez merged commit 3645068 into master Sep 7, 2023
58 checks passed
@isabellaenriquez isabellaenriquez deleted the isabellaenriquez/team-workflow-constraint branch September 7, 2023 17:33
@isabellaenriquez isabellaenriquez added the Trigger: Revert Add to a merged PR to revert it (skips CI) label Sep 7, 2023
@getsentry-bot
Copy link
Contributor

revert failed (conflict? already reverted?) -- check the logs

roggenkemper added a commit that referenced this pull request Sep 12, 2023
this pr adds back the addition of team as a field on GroupSubscription
(originally from #55805 ,
removed in #55870) .
@github-actions github-actions bot locked and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Trigger: Revert Add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add team_id to GroupSubscription
4 participants