Testing for migrations from sys.argv is too naive

[intgr]

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.

[flixx]

Hi @intgr understood.

Probably additionally, something like this could be used to check if we are inside djangos migration module...

import inspect

def is_inside_migration():
    for frame_info in inspect.stack():
        if frame_info[1].endswith("django/db/migrations/autodetector.py"):
            return True
    return False

Not sure if django/db/migrations/autodetector.py would be the best choice though. Do you know a file that is for sure in the stack when running migrations?

[intgr]

I think that won't work. A bigger issue is that with this syntax:

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

The deprecate_field() function is executed at import-time. Imports may come from arbitrary locations, at which time it's not known whether the field is going to be used for accessing data or migrating the schema. Indeed during the life of one Python process, like my test suite, it may be used both ways.

So I'm afraid returning different things based on a condition inside deprecate_field() function is not a good solution. You might have to return DeprecatedField always and do something conditional in there. But I'm guessing this brings more complications.

[christianbundy]

Could we use something simple like an environment variable for this? It's janky, but for folks who have complicated migrations configurations that aren't just ./manage.py migrate it might be useful to have an escape hatch.