Skip to content

Conversation

@mifu67
Copy link
Contributor

@mifu67 mifu67 commented Jan 8, 2025

"Completely defunct model;" no foreign keys constraints. No rows in redash and is not used in the codebase. PR 1 of https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-tables.

@mifu67 mifu67 requested review from cathteng and leedongwei January 8, 2025 22:31
@mifu67 mifu67 requested a review from a team as a code owner January 8, 2025 22:31
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 8, 2025

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

--
-- Moved model AuthProviderDefaultTeams to pending deletion state
--
-- (no-op)

@codecov
Copy link

codecov bot commented Jan 8, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
23445 1 23444 271
View the top 1 failed tests by shortest run time
tests.sentry.backup.test_comparators::test_default_comparators
Stack Traces | 0.399s run time
#x1B[1m#x1B[.../sentry/backup/test_comparators.py#x1B[0m:2162: in test_default_comparators
    insta_snapshot(serialized)
#x1B[1m#x1B[31mE   Failed: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~#x1B[0m
#x1B[1m#x1B[31mE   Snapshot .../snapshots/test_comparators/test_default_comparators.pysnap changed!#x1B[0m
#x1B[1m#x1B[31mE   #x1B[0m
#x1B[1m#x1B[31mE   #x1B[0m
#x1B[1m#x1B[31mE   Re-run pytest with SENTRY_SNAPSHOTS_WRITEBACK=new and then use 'make review-python-snapshots' to review.#x1B[0m
#x1B[1m#x1B[31mE   #x1B[0m
#x1B[1m#x1B[31mE   Or: Use SENTRY_SNAPSHOTS_WRITEBACK=1 to update snapshots directly.#x1B[0m
#x1B[1m#x1B[31mE   #x1B[0m
#x1B[1m#x1B[31mE   #x1B[0m
#x1B[1m#x1B[31mE   --- #x1B[0m
#x1B[1m#x1B[31mE   #x1B[0m
#x1B[1m#x1B[31mE   +++ #x1B[0m
#x1B[1m#x1B[31mE   #x1B[0m
#x1B[1m#x1B[31mE   @@ -244,12 +244,6 @@#x1B[0m
#x1B[1m#x1B[31mE   #x1B[0m
#x1B[1m#x1B[31mE        fields:#x1B[0m
#x1B[1m#x1B[31mE        - organization_id#x1B[0m
#x1B[1m#x1B[31mE      model_name: sentry.authprovider#x1B[0m
#x1B[1m#x1B[31mE   -- comparators:#x1B[0m
#x1B[1m#x1B[31mE   -  - class: ForeignKeyComparator#x1B[0m
#x1B[1m#x1B[31mE   -    fields:#x1B[0m
#x1B[1m#x1B[31mE   -    - authprovider_id#x1B[0m
#x1B[1m#x1B[31mE   -    - team_id#x1B[0m
#x1B[1m#x1B[31mE   -  model_name: sentry.authproviderdefaultteams#x1B[0m
#x1B[1m#x1B[31mE    - comparators:#x1B[0m
#x1B[1m#x1B[31mE      - class: ForeignKeyComparator#x1B[0m
#x1B[1m#x1B[31mE        fields:#x1B[0m
#x1B[1m#x1B[31mE   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

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

Since this is a control model, when deleting you may need to add it to

historical_silo_assignments = {
"sentry_actor": SiloMode.REGION,
"sentry_teamavatar": SiloMode.REGION,
"sentry_projectavatar": SiloMode.REGION,
"sentry_pagerdutyservice": SiloMode.REGION,
"sentry_notificationsetting": SiloMode.CONTROL,
"authprovider_duplicate": SiloMode.CONTROL,
"authidentity_duplicate": SiloMode.CONTROL,
}
in your next pr. You can just create the pr without it at first, then if the generated sql is empty add it in.

Copy link
Member

@cathteng cathteng left a comment

Choose a reason for hiding this comment

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

🥳

@wedamija
Copy link
Member

wedamija commented Jan 9, 2025

If you could hold off merging until #83140 and #83143 are merged then you can remove all these snapshot changes. It's hard for me to get my prs merged, should just be 20-30 mins or so!

Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

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

Ok, good to go. You'll just need to rebase and remove any changes to the json files since they're now deleted

@mifu67 mifu67 force-pushed the mifu67/delete-auth-provider-default-teams branch from 93fbabe to 4ebc0c1 Compare January 9, 2025 19:25
@mifu67 mifu67 force-pushed the mifu67/delete-auth-provider-default-teams branch from 4ebc0c1 to 114f702 Compare January 9, 2025 19:31
@mifu67 mifu67 removed the Scope: Frontend Automatically applied to PRs that change frontend components label Jan 9, 2025
@getsentry getsentry deleted a comment from github-actions bot Jan 9, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jan 9, 2025

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

--
-- Moved model AuthProviderDefaultTeams to pending deletion state
--
-- (no-op)

@mifu67 mifu67 merged commit bc0ca0f into master Jan 9, 2025
47 checks passed
@mifu67 mifu67 deleted the mifu67/delete-auth-provider-default-teams branch January 9, 2025 20:01
mifu67 added a commit that referenced this pull request Jan 10, 2025
Follow-up to #83131. May need to
add this table to the historical silo assignments if the migration is a
no-op.
andrewshie-sentry pushed a commit that referenced this pull request Jan 22, 2025
"Completely defunct model;" no foreign keys constraints. No rows in
redash and is not used in the codebase. PR 1 of
https://develop.sentry.dev/backend/application-domains/database-migrations/#deleting-tables.
andrewshie-sentry pushed a commit that referenced this pull request Jan 22, 2025
Follow-up to #83131. May need to
add this table to the historical silo assignments if the migration is a
no-op.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 25, 2025
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants