getsentry / sentry

Developer-first error tracking and performance monitoring
https://sentry.io
Other
39.23k stars 4.21k forks source link

feat(migrations): Prevent new migrations from using `RunSQL` #81003

Open wedamija opened 2 days ago

wedamija commented 2 days ago

This adds in allow_run_sql to our CheckedMigration baseclass. We want to prevent the use of RunSQL in most cases going forward, since our migration safety framework doesn't know how to parse sql to ensure safety.

Our main uses for this in the past were:

wedamija commented 2 days ago

Note that some tests will fail until we have a way to handle deleting tables/cols

codecov[bot] commented 2 days ago

:x: 1 Tests Failed:

Tests completed Failed Passed Skipped
23090 1 23089 215
View the top 1 failed tests by shortest run time > > ```python > tests.sentry.db.postgres.schema.safe_migrations.integration.test_migrations.DeleteModelCorrectTest::test > ``` > >
Stack Traces | 0.722s run time > > > > > ```python > > #x1B[1m#x1B[.../safe_migrations/integration/test_migrations.py#x1B[0m:215: in test > > self.run_migration() > > #x1B[1m#x1B[.../safe_migrations/integration/test_migrations.py#x1B[0m:38: in run_migration > > self._run_migration(self.app, self.migrate_to) > > #x1B[1m#x1B[.../safe_migrations/integration/test_migrations.py#x1B[0m:48: in _run_migration > > executor.migrate(target) > > #x1B[1m#x1B[31m.venv/lib/python3.12.../db/migrations/executor.py#x1B[0m:135: in migrate > > state = self._migrate_all_forwards( > > #x1B[1m#x1B[31m.venv/lib/python3.12.../db/migrations/executor.py#x1B[0m:167: in _migrate_all_forwards > > state = self.apply_migration( > > #x1B[1m#x1B[.../new_migrations/monkey/executor.py#x1B[0m:141: in apply_migration > > return super().apply_migration(state, migration, fake=fake, fake_initial=fake_initial) > > #x1B[1m#x1B[31m.venv/lib/python3.12.../db/migrations/executor.py#x1B[0m:255: in apply_migration > > state = migration.apply(state, schema_editor) > > #x1B[1m#x1B[.../sentry/new_migrations/migrations.py#x1B[0m:30: in apply > > raise UnsafeOperationException( > > #x1B[1m#x1B[31mE django_zero_downtime_migrations.backends.postgres.schema.UnsafeOperationException: Using RunSQL is unsafe because our migrations safety framework can't detect problems with the migration. If you need to use RunSQL, set `allow_run_sql = True` and get approval from `owners-migrations` to make sure that it's safe.#x1B[0m > > ``` > >

To view more test analytics, go to the Test Analytics Dashboard Got feedback? Let us know on Github