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

fix: address django migration failing on SQLite #5308

Merged
merged 8 commits into from
Nov 28, 2024

Conversation

joeyorlando
Copy link
Contributor

@joeyorlando joeyorlando commented Nov 28, 2024

Which issue(s) this PR closes

Fixes #5306 (and related to #5244 (comment)) + add CI check to avoid this from happening in the future

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:patch PR will be added to "Other Changes" section of release notes labels Nov 28, 2024
@joeyorlando joeyorlando requested a review from a team as a code owner November 28, 2024 16:41
Comment on lines +15 to +24
# NOTE: commented out due to some issues this was causing w/ SQLite:
# https://github.com/grafana/oncall/issues/5306
# https://github.com/grafana/oncall/issues/5244#issuecomment-2503999986
#
# linter.IgnoreMigration(),
# common.migrations.remove_field.RemoveFieldDB(
# model_name='resolutionnoteslackmessage',
# name='_slack_channel_id',
# remove_state_migration=('alerts', '0068_remove_resolutionnoteslackmessage__slack_channel_id_state'),
# ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

honestly I don't think there is any harm in commenting this out. It's really only doing a DROP COLUMN statement.

For whatever reason, this seems to fail (only on SQLite; see #5306 for more details):

oncall_db_migration-1  | django.db.utils.OperationalError: error in index alerts_reso_ts_a9bdf7_idx after drop column: no such column: _slack_channel_id
oncall_db_migration-1  |   Applying alerts.0070_remove_resolutionnoteslackmessage__slack_channel_id_db...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before

😢

Screenshot 2024-11-28 at 12 27 48 PM

After

🙂

Screenshot 2024-11-28 at 12 39 54 PM

@joeyorlando joeyorlando added this pull request to the merge queue Nov 28, 2024
@joeyorlando joeyorlando removed this pull request from the merge queue due to a manual request Nov 28, 2024
@joeyorlando joeyorlando merged commit 86ca438 into dev Nov 28, 2024
26 checks passed
@joeyorlando joeyorlando deleted the jorlando/fix-migration-failing-on-sqlite branch November 28, 2024 18:05
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:patch PR will be added to "Other Changes" section of release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New Installation with sqlite fails migrations
2 participants