ref(hc): Move PagerDutyService into control silo #53084

corps · 2023-07-18T17:12:51Z

Notably, this migration is a net zero SQL, it only has to change the state of these columns in django.

I'll want to add the constraint on organization_integration incrementally as a separate PR for 2 reasons

It is unsafe to do so synchronously -- it will be plausible that there are some organization_integration_ids that don't point to existing rows.
Related to ^^ but since this column was a HybridCloudForeignKey, it means the cascade was applied asynchronously for deletes. The goal will be to roll this out, making future deletes stop cascading, then manually clean up any objects pointing to missing OIs, then roll out the migration that adds the constraint back in.

github-actions · 2023-07-18T17:15:35Z

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

--
-- Custom state/database change combination
--

codecov · 2023-07-18T17:42:01Z

Codecov Report

Merging #53084 (fe9bc12) into master (4ec75b8) will increase coverage by 0.00%.
The diff coverage is 90.47%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #53084   +/-   ##
=======================================
  Coverage   79.51%   79.51%           
=======================================
  Files        4938     4939    +1     
  Lines      207934   207971   +37     
  Branches    35473    35478    +5     
=======================================
+ Hits       165335   165372   +37     
+ Misses      37566    37559    -7     
- Partials     5033     5040    +7

Impacted Files	Coverage Δ
src/sentry/integrations/pagerduty/integration.py	`95.74% <ø> (ø)`
...terfaces/crashContent/exception/sourceMapDebug.tsx	`81.13% <ø> (+1.50%)`	⬆️
...faces/crashContent/exception/useSourceMapDebug.tsx	`88.23% <ø> (ø)`
...ummary/transactionOverview/durationChart/index.tsx	`65.00% <ø> (-4.57%)`	⬇️
...app/views/performance/transactionSummary/utils.tsx	`81.08% <ø> (ø)`
...try/integrations/pagerduty/actions/notification.py	`77.77% <73.33%> (-0.56%)`	⬇️
.../serializers/rest_framework/notification_action.py	`82.66% <100.00%> (+0.11%)`	⬆️
src/sentry/integrations/pagerduty/actions/form.py	`82.92% <100.00%> (+2.92%)`	⬆️
...rc/sentry/models/integrations/pagerduty_service.py	`100.00% <100.00%> (ø)`
...c/sentry/services/hybrid_cloud/integration/impl.py	`84.30% <100.00%> (ø)`
... and 2 more

... and 25 files with indirect coverage changes

src/sentry/utils/query.py

github-actions · 2023-07-18T20:15:45Z

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

--
-- Custom state/database change combination
--

GabeVillalobos · 2023-07-19T23:31:23Z

src/sentry/integrations/pagerduty/actions/form.py

-
-        integration = integration_service.get_integration(
-            organization_integration_id=service.organization_integration_id
+        ois = integration_service.get_organization_integrations(


nit: I find these 2-3 letter variable names harder to follow than a more verbose variable at least denoting the type

GabeVillalobos · 2023-07-19T23:32:54Z

src/sentry/integrations/pagerduty/actions/form.py

+            pds
+            for oi in ois
+            for pds in oi.config.get("pagerduty_services", [])
+            if pds["id"] == service_id


is it possible to have more than one match per service_id? If so is this expected/valid behavior?

Nope, there should only be one -- the service_id is a global unique identifier from the PagerDutyService object. In the future, we'll likely generate these by other means when we drop the model all together.

GabeVillalobos · 2023-07-19T23:40:46Z

src/sentry/models/integrations/pagerduty_service.py

    __include_in_export__ = False

-    organization_integration_id = HybridCloudForeignKey(
-        "sentry.OrganizationIntegration", on_delete="CASCADE"
+    # organization_integration_id = HybridCloudForeignKey(


Leftover comments?

GabeVillalobos · 2023-07-19T23:45:41Z

src/sentry/services/hybrid_cloud/integration/serial.py

+    # if oi.integration
+    # i for i in integrations if i.provider == ExternalProviders.PAGERDUTY.name
+    # if


More leftover comments?

Correct, I'll clean up

src/sentry/services/hybrid_cloud/integration/impl.py

src/sentry/migrations/0516_switch_pagerduty_silo.py

getsentry-bot · 2023-07-20T03:05:33Z

PR reverted: 7e86a48

This reverts commit 4840bb4. Co-authored-by: corps <593850+corps@users.noreply.github.com>

This is #53084 but with the additional commit 7c0ad04 due to the issue in production caused by bad tests that don't simulate the form behavior.

ref(hc): Move PagerDutyService into control silo

d6a4b82

corps requested a review from a team July 18, 2023 17:12

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 18, 2023

markstory reviewed Jul 18, 2023

View reviewed changes

src/sentry/utils/query.py Outdated Show resolved Hide resolved

Thing

11ecd85

vercel bot deployed to Preview July 18, 2023 19:39 View deployment

Merge branch 'master' into zc/switch-pagerduty-silo

52c3bde

vercel bot deployed to Preview July 18, 2023 20:15 View deployment

More fixes

5d9f676

vercel bot deployed to Preview July 19, 2023 00:27 View deployment

Fix

78ab773

vercel bot deployed to Preview July 19, 2023 00:51 View deployment

More fixes

92cf032

vercel bot deployed to Preview July 19, 2023 01:32 View deployment

corps and others added 2 commits July 19, 2023 13:07

Merge branch 'master' into zc/switch-pagerduty-silo

26bb289

Yup

5bf2f57

vercel bot deployed to Preview July 19, 2023 23:11 View deployment

corps marked this pull request as ready for review July 19, 2023 23:21

corps requested review from a team as code owners July 19, 2023 23:21

corps requested a review from a team July 19, 2023 23:21

Make PagerDutyService temporarily dual silo

07a6744

vercel bot deployed to Preview July 19, 2023 23:46 View deployment

GabeVillalobos reviewed Jul 19, 2023

View reviewed changes

Address feedback + test fixes

fe9bc12

vercel bot deployed to Preview July 20, 2023 00:48 View deployment

GabeVillalobos approved these changes Jul 20, 2023

View reviewed changes

corps merged commit 4840bb4 into master Jul 20, 2023

corps deleted the zc/switch-pagerduty-silo branch July 20, 2023 01:26

corps added the Trigger: Revert Add to a merged PR to revert it (skips CI) label Jul 20, 2023

getsentry-bot added a commit that referenced this pull request Jul 20, 2023

Revert "ref(hc): Move PagerDutyService into control silo (#53084)"

7e86a48

This reverts commit 4840bb4. Co-authored-by: corps <593850+corps@users.noreply.github.com>

corps mentioned this pull request Jul 20, 2023

ref(hc): PageDurtyService silo swap #53250

Merged

corps added a commit that referenced this pull request Jul 20, 2023

ref(hc): PageDurtyService silo swap (#53250)

a35e3e0

This is #53084 but with the additional commit 7c0ad04 due to the issue in production caused by bad tests that don't simulate the form behavior.

github-actions bot locked and limited conversation to collaborators Aug 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ref(hc): Move PagerDutyService into control silo #53084

ref(hc): Move PagerDutyService into control silo #53084

corps commented Jul 18, 2023

github-actions bot commented Jul 18, 2023

codecov bot commented Jul 18, 2023 •

edited

Loading

github-actions bot commented Jul 18, 2023 •

edited

Loading

GabeVillalobos Jul 19, 2023

GabeVillalobos Jul 19, 2023

corps Jul 20, 2023

GabeVillalobos Jul 19, 2023

GabeVillalobos Jul 19, 2023

corps Jul 20, 2023

getsentry-bot commented Jul 20, 2023

ref(hc): Move PagerDutyService into control silo #53084

ref(hc): Move PagerDutyService into control silo #53084

Conversation

corps commented Jul 18, 2023

github-actions bot commented Jul 18, 2023

codecov bot commented Jul 18, 2023 • edited Loading

Codecov Report

github-actions bot commented Jul 18, 2023 • edited Loading

GabeVillalobos Jul 19, 2023

Choose a reason for hiding this comment

GabeVillalobos Jul 19, 2023

Choose a reason for hiding this comment

corps Jul 20, 2023

Choose a reason for hiding this comment

GabeVillalobos Jul 19, 2023

Choose a reason for hiding this comment

GabeVillalobos Jul 19, 2023

Choose a reason for hiding this comment

corps Jul 20, 2023

Choose a reason for hiding this comment

getsentry-bot commented Jul 20, 2023

codecov bot commented Jul 18, 2023 •

edited

Loading

github-actions bot commented Jul 18, 2023 •

edited

Loading