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

Add instruction on removing nullable fields from Django models #2659

Merged
merged 8 commits into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions dev/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -427,3 +427,32 @@ backwards compatible

See [django-migration-linter checklist](https://github.com/3YOURMIND/django-migration-linter/blob/main/docs/incompatibilities.md)
for the common mistakes and best practices

### Removing a nullable field from a model
Copy link
Member Author

@vstpme vstpme Jul 27, 2023

Choose a reason for hiding this comment

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

See preview here


> This only works for nullable fields (fields with `null=True` in the field definition).
>
> DO NOT USE THIS APPROACH FOR NON-NULLABLE FIELDS, IT CAN BREAK THINGS!

1. Remove all usages of the field you want to remove. Make sure the field is not used anywhere, including filtering,
querying, or explicit field referencing from views, models, forms, serializers, etc.
2. Remove the field from the model definition.
3. Generate migrations using the following management command:

```python
python manage.py remove_field <APP_LABEL> <MODEL_NAME> <FIELD_NAME>
```

Example: `python manage.py remove_field alerts AlertReceiveChannel restricted_at`

This command will generate two migrations that **MUST BE DEPLOYED IN TWO SEPARATE RELEASES**:
- Migration #1 will remove the field from Django's state, but not from the database. Release #1 must include
migration #1, and must not include migration #2.
- Migration #2 will remove the field from the database. Stash this migration for use in a future release.

4. Make release #1 (removal of the field + migration #1). Once released and deployed, Django will not be
aware of this field anymore, but the field will be still present in the database. This allows for a gradual migration,
where the field is no longer used in new code, but still exists in the database for backward compatibility with old code.
5. In any subsequent release, include migration #2 (the one that removes the field from the database).
6. After releasing and deploying migration #2, the field will be removed both from the database and Django state,
without backward compatibility issues or downtime 🎉
Empty file.
79 changes: 79 additions & 0 deletions engine/common/migrations/remove_field.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
from django.db import connection
from django.db.migrations import RemoveField
from django.db.migrations.loader import MigrationLoader


class RemoveFieldState(RemoveField):
"""
Remove field from Django's migration state, but not from the database.
This is essentially the same as RemoveField, but database_forwards and database_backwards methods are modified
to do nothing.
"""

def database_forwards(self, app_label, schema_editor, from_state, to_state):
pass

def database_backwards(self, app_label, schema_editor, from_state, to_state):
pass

def describe(self):
return f"{super().describe()} (state)"

@property
def migration_name_fragment(self):
return f"{super().migration_name_fragment}_state"


class RemoveFieldDB(RemoveField):
"""
Remove field from the database, but not from Django's migration state.
This is implemented as a custom operation, because Django's RemoveField operation does not support
removing fields from the database after it has been removed from the state. The workaround is to use the state
that was in effect before the field was removed from the state (i.e. just before the RemoveFieldState migration).
"""

def __init__(self, model_name, name, remove_state_migration):
"""
Specifying "remove_state_migration" allows database operations to run against a particular historical state.
Example: remove_state_migration = ("alerts", "0014_alertreceivechannel_restricted_at") will "trick" Django
into thinking that the last applied migration in the "alerts" app is 0013.
"""
super().__init__(model_name, name)
self.remove_state_migration = remove_state_migration

def deconstruct(self):
"""Update serialized representation of the operation."""
deconstructed = super().deconstruct()
return (
deconstructed[0],
deconstructed[1],
deconstructed[2] | {"remove_state_migration": self.remove_state_migration}
)

def state_forwards(self, app_label, state):
"""Skip any state changes."""
pass

def database_forwards(self, app_label, schema_editor, from_state, to_state):
# use historical state instead of what Django provides
from_state = self.state_before_remove_state_migration

super().database_forwards(app_label, schema_editor, from_state, to_state)

def database_backwards(self, app_label, schema_editor, from_state, to_state):
# use historical state instead of what Django provides
to_state = self.state_before_remove_state_migration

super().database_backwards(app_label, schema_editor, from_state, to_state)

def describe(self):
return f"{super().describe()} (db)"

@property
def migration_name_fragment(self):
return f"{super().migration_name_fragment}_db"

@property
def state_before_remove_state_migration(self):
"""Get project state just before migration "remove_state_migration" was applied."""
return MigrationLoader(connection).project_state(self.remove_state_migration, at_end=False)
64 changes: 64 additions & 0 deletions engine/engine/management/commands/remove_field.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
from django.core.management import BaseCommand
from django.db import connection
from django.db.migrations import Migration
from django.db.migrations.autodetector import MigrationAutodetector
from django.db.migrations.loader import MigrationLoader
from django.db.migrations.writer import MigrationWriter

from common.migrations.remove_field import RemoveFieldDB, RemoveFieldState


class Command(BaseCommand):
Copy link
Member Author

Choose a reason for hiding this comment

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

Example: python manage.py remove_field alerts AlertReceiveChannel restricted_at (assuming that field exists)

This generates two migrations:

0031_remove_alertreceivechannel_restricted_at_state.py

# Generated by Django 3.2.20 on 2023-08-07 20:26

import common.migrations.remove_field
from django.db import migrations


class Migration(migrations.Migration):

    dependencies = [
        ('alerts', '0030_auto_20230731_0341'),
    ]

    operations = [
        common.migrations.remove_field.RemoveFieldState(
            model_name='AlertReceiveChannel',
            name='restricted_at',
        ),
    ]

0032_remove_alertreceivechannel_restricted_at_db.py

# Generated by Django 3.2.20 on 2023-08-07 20:26

import common.migrations.remove_field
from django.db import migrations


class Migration(migrations.Migration):

    dependencies = [
        ('alerts', '0031_alertreceivechannel_restricted_at'),
    ]

    operations = [
        common.migrations.remove_field.RemoveFieldDB(
            model_name='AlertReceiveChannel',
            name='restricted_at',
            remove_state_migration=('alerts', '0031_remove_alertreceivechannel_restricted_at_state'),
        ),
    ]

"""
Generate two migrations that remove a field from the state and the database separately.
This allows removing a field in 2 separate releases and avoid downtime.
"""

def add_arguments(self, parser):
parser.add_argument(
"args", nargs=3, help="app_label model_name field_name, example: alerts AlertReceiveChannel restricted_at"
)

def handle(self, *args, **options):
app_label, model_name, field_name = args

# Check that the app, the model, and the field to be removed exist
project_state = MigrationLoader(connection).project_state()
model_state = project_state.apps.get_model(app_label, model_name)
model_state._meta.get_field(field_name)

# Write migration that removes the field from the state
remove_state_migration = self.write_operation(
app_label, RemoveFieldState(model_name=model_name, name=field_name), project_state
)

# Write migration that removes the field from the database
self.write_operation(
app_label,
RemoveFieldDB(
model_name=model_name, name=field_name, remove_state_migration=(app_label, remove_state_migration.name)
),
project_state,
)

@staticmethod
def write_operation(app_label, operation, project_state):
"""
Some Django magic to write a single-operation migration to a file, so it's similar to what Django would generate
when running the "makemigrations" command.
"""

migration_class = type("Migration", (Migration,), {"operations": [operation]})

changes = MigrationAutodetector(project_state, project_state).arrange_for_graph(
changes={app_label: [migration_class(None, app_label)]},
graph=MigrationLoader(connection).graph,
migration_name=operation.migration_name_fragment,
)

migration = changes[app_label][0]
writer = MigrationWriter(migration)
with open(writer.path, "w", encoding="utf-8") as file:
file.write(writer.as_string())

return migration