#22983 closed Bug (fixed)
Invalid import in a squashed migration of migrations having RunPython
Reported by: | Piotr Maliński | Owned by: | Andrew Godwin |
---|---|---|---|
Component: | Migrations | Version: | 1.7-rc-1 |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.
Change History (14)
comment:1 by , 10 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
Version: | 1.6 → 1.7-rc-1 |
comment:2 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 10 years ago
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?
comment:4 by , 10 years ago
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?
comment:5 by , 10 years ago
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)
follow-up: 8 comment:7 by , 10 years ago
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?
comment:8 by , 10 years ago
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.
comment:9 by , 10 years ago
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.
comment:10 by , 10 years ago
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
comment:11 by , 10 years ago
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.
comment:12 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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.