3yourmind/django-deprecate-fields

This package allows deprecating model fields and allows removing them in a backwards compatible manner.

Django - Deprecate Field

Installation

pip install django-deprecate-fields

Usage

Assume the simple following model:

from django.db import models

class MyModel(models.Model):
    field1 = models.CharField()
    field2 = models.CharField()

In order to remove field1, it should first be marked as deprecated:

from django.db import models
from django_deprecate_fields import deprecate_field

class MyModel(models.Model):
    field1 = deprecate_field(models.CharField())
    field2 = models.CharField()

Secondly, makemigrations should be called, which will change the field to be nullable. Any lingering references to it in your code will return None (or optionally any value or callable passed to deprecate_field as the return_instead argument)

Lastly, after the changes above have been deployed, field1 can then safely be removed in the model (plus another makemigrations run)

Contributing

First of all, thank you very much for contributing to this project. Please base your work on the master branch and target master in your pull request.

License

django-deprecate-fields is released under the Apache 2.0 License.

django-deprecate-fields's Issues

deprecate_field does unnecessary conversion to NullBooleanField

Hi there, Django supports BooleanField(null=True) since Django 2.1 - https://docs.djangoproject.com/en/2.1/ref/models/fields/#django.db.models.BooleanField

If you try to deprecate one of these through the library, it ends up creating a no-op migration to switch the field to a NullBooleanField. It would be better to detect this scenario and not create a migration in this instance.

Potential for errors when deprecating non-nullable fields

Say in version N of my project I have this model and code that assumes foo_instance.text_field is always going to be a string.

class FooModel(models.Model):
    text_field = models.CharField(max_length=256)

Then in version N+1 I then deprecate it

class FooModel(models.Model):
    text_field = deprecate_field(models.CharField(max_length=256), "")

Then when version N and N+1 are running simultaneously:

1. version N+1 inserts a row, it will have text_field=NULL
2. version N selects this row, and assumes text_field is going to be a string, when it is actually None
3. Potentially raising a TypeError / ValueError

I'm not sure how this could be addressed by the django-deprecate-fields library. Maybe this just needs to be detailed in the docs.

For us the solution is to first use a AddDefaultValue operation on the field to add a database-level default. This is so the rows inserted by N+1 get this default value instead of NULL, so version N can still handle them.

Flake8 extension?

Has there been a discussion around making a flake8 extension to raise issues if a deprecated_field is referenced?

Logger should be instantiated with `__name__`

The logger in deprecate_field.py is instantiated with __file__ which makes the logger uncontrollable from the LOGGING setting. Could it be changed to __name__ ?

Should we add `IgnoreMigration()`into the second step migration?

The question is about the second step:

Lastly, after the changes above have been deployed, field1 can then safely be removed in the model (plus another makemigrations run)

When we remove a field that was deprecated in the previous release by adding deprecate_field(), should we add IgnoreMigration() into migration where we remove the field in the second step?

Deprecate a ManyToMany field tested with TransactionTestCase

In short: using deprecate_field on ManyToMany field and then creating the model inside TransactionTestCase raises ERROR: cannot truncate a table referenced in a foreign key constraint (at least in Postgres).

Explanation:
Django's TransactionTestCase uses truncate to flush the DB after each test. truncate is called on all existing tables simultaneously to avoid issues with any of them having an FK to another (in Postgres: “truncate cannot be used on a table that has FK references from other tables, unless all such tables are also truncated in the same command”). Django searches for table names by accessing Model._meta.db_table and Model._meta.local_many_to_many . But since deprecate_field makes the field getter return None, truncate isn’t called on the table that stores many-to-many relationship. Consequently, a DB error is raised.

This sounds like a pretty niche case and not sure if it's even worth fixing, since quite a lot of stars have to align for you to experience this. Nonetheless, I decided to share to spare others the pain of debugging.

deprecate_field: accessing deprecated field NoneType.<unknown>

We started used deprecate-fields and saw this warning in our logs.

Turns out django-fsm uses inspect.getmembers which in turn does a getattr(object, key) where the object is the class, not an instance.

Maybe simply catching the None value at the start would be enough.

Tests

Hey guy, awesome work with this package. It's making our deployments zero downtime.
I'm thinking about adding tests since I couldn't find any.
Any suggestions?

Regards,

Andre

Handle 0-downtime deploys with high-read-frequency models

I have a deploy process that runs the migration then rolls out the new app version to our codebase. As a result, there is a period of time where the old version of the model is querying the database after the migration deleting the column has been run. This leads to errors like the following whenever the model with the removed column is read from the DB in the older version of the code:

column TABLE_NAME.COLUMN_NAME does not exist
LINE 1: ...

I think this medium post describes part of a potential solution. I think the solution would involve extending the migration logic to add some kind of noop that allows us to remove the column from the model in the app before removing in the migration, while tricking makemigrations into thinking that everything is in a legal state.

Deprecate a whole model?

Do you have any experience deprecating a whole model, not just a field? What I tried is to deprecate and then remove every field, so now I'm left with:

class MyModel(models.Model):
    pass

Still, this for some reason makes the Django ORM still issue queries in the form of SELECT "my_model"."id" FROM "my_model" ORDER BY "my_model"."id" ASC, seemingly for every test that is marked with the @pytest.mark.django_db decorator.

(Background: The CI pipeline is making a migration to the latest DB version, i.e. a revision where I dropped MyModel, and then executing the test suite of the previous software version, i.e. with the model still in place, to make sure that the previous version can run on the latest schema. This enforces us to always use deprecate_field(...) before dropping a column, but now I'm left without a clue how to apply this on a model level)

Testing for migrations from sys.argv is too naive

Currently this project uses the following test to check whether we're migrating:

    if not set(sys.argv) & {"makemigrations", "migrate", "showmigrations"}:
        return DeprecatedField(return_instead)

This check can be too naive, however:

• We directly call MigrationAutodetector in our test suite to ensure no missing migrations, but that fails if we add deprecate_field() anywhere.
• We have a custom manage.py deploy command which among other things calls call_command("migrate", interactive=False). This now prints warnings:

Your models in app(s): '...' have changes that are not yet reflected in a migration, and so won't be applied.

I haven't looked into better solutions yet, just letting you know of this problem.

Noisy errors on startup

Hi 👋

Since implementing deprecate_field we've started seeing this output whenever we initialize django:

accessing deprecated field NoneType.<unknown>
accessing deprecated field NoneType.<unknown>

After a lot of digging I have not been able to get to the bottom of exactly where our deprecated field is called, but it seems to be indirect. There are no direct calls to the deprecated field anywhere.

Would you accept a PR where we change the default behavior to only outputting this if either the class name or object name is known?

Deprecate a field that is a Foreign key

Hi
We added the deprecate_field to a foreign key field and it doesn't generate a migration for the change.
Is it related to the foreign key or because it's Django 3.2.9?
Here is the sample code:

    category = deprecate_field(
        models.ForeignKey(
            ModelCategory,
            on_delete=models.SET_NULL,
            null=True,
            blank=True,
            db_index=True,
            verbose_name=("Category (Deprecated)"),
        )
    )

We are already good to remove the field but we were following the recommended on https://github.com/3YOURMIND/django-migration-linter/blob/main/docs/incompatibilities.md#arrow_forward-dropping-a-column
But we can't make the linter happy. We wonder if it's because of the missing migration to deprecate the field.

An option to raise an exception when a deprecated field is accessed

Kind of additional argument raise_on_access: bool = False to the deprecate_field() function, that will instead of showing a warning, raise an exception when this field is somehow accessed.

New release?

Hi, this package is a great idea, good work!

Could you do new release, since the latest on pypi is 0.0.3?
Also, could you tag your releases?