Invalid import in a squashed migration of migrations having RunPython

[Piotr Maliński]

I've squashed migrations in one of my apps and I've noticed that every migration that had a custom function on RunPython resulted in an invalid import in the squashed migration like:

import my_app.migrations.0010_clean_propertyindex

and then it tries to use it:

migrations.RunPython(
            code=my_app.migrations.0010_clean_propertyindex.delete_indexes,
            reverse_code=None,
            atomic=True,
        ),

It's either a bug or all functions should not be in a file that starts with a digit. Either way squash should fail if it would have to make such import.

[Andrew Godwin]

Ah, this is a tricky one; we're not able to squash the custom functions, as they're not serialisable. This is possibly why I had initially planned RunPython to take strings rather than function objects.

I don't want to make squash hard fail on this kind of error, though, as the fix from the result to a working version is easy (just copy the functions over), whereas squashing manually is almost impossible to do quickly or easily. I think I'll make it leave the function references in place, but remove the valid imports and have squashmigrations output that you need to copy the functions over.

[Claude Paroz]

Is the problem mainly caused by migration files starting with a number? I have already been bitten by this once. I know it's late, but is it still the time to ask if the migration files could/should be prefixed with a letter to make them importable?

[Piotr Maliński]

That output will be essential as developers will be using it a lot and it's a new feature which is not yet known well (so it should be a bit verbose).

Side problem is that RunPython limits squashing efficiency of model changes etc. made after migrations with it. Can squash can be squashed after refactoring order of RunPythons in the squash?

[Andrew Godwin]

claudep: No, the problem is not the number prefix; the old files are meant to be deleted anyway, not imported from. I know it's slightly wrong to have python files starting with a number but I kind of like that you can't import them; they're not meant to be a library resource, and there's no reason you should ever import one by name (rather than loading them dynamically like Django does)

You can, indeed, re-squash a squash once you've reordered RunPython operations in it, but you'll need to go through the whole squash - apply squash - delete old migrations dance first before squashing a second time. There might be room for an in-place optimize command that just optimizes a single file, but that would have to be a 1.8 feature at this point (one could use the Django migration APIs to write that command reasonably easily anyway)

[Claude Paroz]

Andrew: note taken, thanks!

[Matthew Schinckel]

Could we not do something within django.db.migrations.writer.MigrationWriter, that looks at the path of the operation, and rewrites the function definition into the new migration file if the function is defined in the same file as the migration was loaded from?

[Simon Charette]

Replying to schinckel:

Could we not do something within django.db.migrations.writer.MigrationWriter, that looks at the path of the operation, and rewrites the function definition into the new migration file if the function is defined in the same file as the migration was loaded from?

I guess we could use inspect.getsource for this but it wouldn't take non local objects references (imports the function depends on for example) into account.

[Andrew Godwin]

No, as there is no way to cleanly get a function's source code given a pointer to it. You can try using inspect, but you'd need to also take into account any custom imports in the file, any global variables, any extra functions, if the callable is actually a class, and a number of other issues - that is, if you can even read the migration file in the first place (which is likely in this case, though, as you're writing another one, but it's possible someone's compiled everything to .pyc and removed the python files)

The simplest way is to just ask the developer to port it over, as they're in the best position to be able to notice and correct for all of these things.

[generalov]

I put all data migration functions in separate module migrations/datamigrations/__init__.py then import code from it to migration module.

# 0002_some_migration.py 
from . import datamigrations

class Migration(migrations.Migration):
    operations = [
        migrations.RunPython(datamigrations.fix_data),

In this case the squashmigrations builds the following result:

import project.app.migrations.datamigrations

class Migration(migrations.Migration):
    operations = [
        migrations.RunPython(
            code=project.app.migrations.datamigrations.fill_serial,
            reverse_code=None,
            atomic=True,
        ),

The code looks bit ugly but it works without any modifications.

I create datamigration as package instead of just file module beacause otherwise manage.py squashmigrations .. fails with exception django.db.migrations.loader.BadMigrationError: Migration datamigrations in app app has no Migration class

[Andrew Godwin]

Yes, I guess that works. The reason you get the error is because any python file in the migrations directory is assumed to be a migration. Not sure I'd recommend that pattern for people's general use, though, I like encouraging having all the implementation in one file for easier review.

[Andrew Godwin <andrew@…>]

In ebb13bbd884d8c3053d1d342ef0423240feb05e6:

Fixed #22983: Alert when squashing RunPython operations with referred functions.

[Andrew Godwin <andrew@…>]

In 563046a7de24d5e1eac1d455ec5530d0479186e0:

[1.7.x] Fixed #22983: Alert when squashing RunPython operations with referred functions.

[GitHub <noreply@…>]

In 2d07e1aa:

Refs #22983 -- Added tests for squashing migrations with functions from migration files.

Follow up to ebb13bbd884d8c3053d1d342ef0423240feb05e6.