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

chore: patch recent migration files that drop db columns #5277

Merged
merged 8 commits into from
Nov 20, 2024

Conversation

joeyorlando
Copy link
Contributor

@joeyorlando joeyorlando commented Nov 20, 2024

What this PR does

  • patch recent migration files which drop several deprecated DB columns which led to a recent (minor/internal) issue
  • add a CI job to prevent this from happening in the future and instead, force folks to drop columns the proper way (which we have documented internally)

(as documented here, I have the four additional migration files (which actually do the DROP COLUMNs in the db) saved locally, and will include these in a separate PR/release)

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • Added the relevant release notes label (see labels prefixed w/ release:). These labels dictate how your PR will
    show up in the autogenerated release notes.

@joeyorlando joeyorlando added pr:no public docs Added to a PR that does not require public documentation updates release:ignore PR will not be added to release notes labels Nov 20, 2024
@joeyorlando joeyorlando requested a review from a team as a code owner November 20, 2024 20:33
Comment on lines -15 to -18
migrations.RemoveField(
model_name='channelfilter',
name='_slack_channel_id',
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to engine/apps/alerts/migrations/0067_remove_channelfilter__slack_channel_id_state.py

Comment on lines -19 to -22
migrations.RemoveField(
model_name='resolutionnoteslackmessage',
name='_slack_channel_id',
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to engine/apps/alerts/migrations/0068_remove_resolutionnoteslackmessage__slack_channel_id_state.py

Copy link
Contributor Author

@joeyorlando joeyorlando Nov 20, 2024

Choose a reason for hiding this comment

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

example of when this passes:

Checking engine/apps/alerts/migrations/0066_remove_channelfilter__slack_channel_id_and_more.py for migrations.RemoveField...
✅ No RemoveField found in engine/apps/alerts/migrations/0066_remove_channelfilter__slack_channel_id_and_more.py
Checking engine/apps/alerts/migrations/0067_remove_channelfilter__slack_channel_id_state.py for migrations.RemoveField...
✅ No RemoveField found in engine/apps/alerts/migrations/0067_remove_channelfilter__slack_channel_id_state.py
Checking engine/apps/alerts/migrations/0068_remove_resolutionnoteslackmessage__slack_channel_id_state.py for migrations.RemoveField...
✅ No RemoveField found in engine/apps/alerts/migrations/0068_remove_resolutionnoteslackmessage__slack_channel_id_state.py
Checking engine/apps/schedules/migrations/0020_remove_oncallschedule_channel_state.py for migrations.RemoveField...
✅ No RemoveField found in engine/apps/schedules/migrations/0020_remove_oncallschedule_channel_state.py
Checking engine/apps/user_management/migrations/0028_remove_organization_general_log_channel_id_state.py for migrations.RemoveField...
✅ No RemoveField found in engine/apps/user_management/migrations/0028_remove_organization_general_log_channel_id_state.py

example of when it catches a failure:

Checking engine/apps/alerts/migrations/0066_remove_channelfilter__slack_channel_id_and_more.py for migrations.RemoveField...
✅ No RemoveField found in engine/apps/alerts/migrations/0066_remove_channelfilter__slack_channel_id_and_more.py
Checking engine/apps/alerts/migrations/0067_remove_channelfilter__slack_channel_id_state.py for migrations.RemoveField...
✅ No RemoveField found in engine/apps/alerts/migrations/0067_remove_channelfilter__slack_channel_id_state.py
Checking engine/apps/alerts/migrations/0068_remove_resolutionnoteslackmessage__slack_channel_id_state.py for migrations.RemoveField...
✅ No RemoveField found in engine/apps/alerts/migrations/0068_remove_resolutionnoteslackmessage__slack_channel_id_state.py
Checking engine/apps/schedules/migrations/0020_remove_oncallschedule_channel_state.py for migrations.RemoveField...
✅ No RemoveField found in engine/apps/schedules/migrations/0020_remove_oncallschedule_channel_state.py
Checking engine/apps/user_management/migrations/0028_remove_organization_general_log_channel_id_state.py for migrations.RemoveField...
✅ No RemoveField found in engine/apps/user_management/migrations/0028_remove_organization_general_log_channel_id_state.py
Checking engine/apps/user_management/migrations/0029_joey_test.py for migrations.RemoveField...
❌ Error: Found migrations.RemoveField in engine/apps/user_management/migrations/0029_joey_test.py
❌ Error: Found migrations.RemoveField in one or more migration files. Please check out our documentation at https://github.com/grafana/oncall/tree/dev/dev#removing-a-nullable-field-from-a-model on how to properly drop columns.

@joeyorlando joeyorlando merged commit dd65732 into dev Nov 20, 2024
26 checks passed
@joeyorlando joeyorlando deleted the jorlando/patch-recent-migration-files branch November 20, 2024 22:02
joeyorlando added a commit that referenced this pull request Nov 21, 2024
# What this PR does

Second, and final part of, #5277

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] Added the relevant release notes label (see labels prefixed w/
`release:`). These labels dictate how your PR will
    show up in the autogenerated release notes.
@VermiumSifell VermiumSifell mentioned this pull request Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates release:ignore PR will not be added to release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant