:rocket: Detect backward incompatible migrations for your django project
Home Page: https://pypi.python.org/pypi/django-migration-linter/
License: Apache License 2.0
![dependabot[bot] avatar](https://avatars.githubusercontent.com/in/29110?v=4 "dependabot[bot]")
I have a model as such, where I am adding ONLY the description field. Which should allow null and blank values:
class BlogPost(models.Model):
"""
This model is used just as an example.
With it we show how one can:
- Use fixtures and factoris
- Use migrations testing
"""
title = models.CharField(max_length=_POST_TITLE_MAX_LENGTH)
body = models.TextField()
# only this line is new
description = models.TextField(null=True, blank=True)
created_at = models.DateTimeField(auto_now_add=True)
modifiet_at = models.DateTimeField(auto_now=True)
def __str__(self) -> str:
"""All django models should have this method."""
return textwrap.wrap(self.title, _POST_TITLE_MAX_LENGTH // 4)[0]and running makemigrations creates this migration:
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('main', '0001_initial'),
]
operations = [
migrations.AddField(
model_name='blogpost',
name='description',
field=models.TextField(blank=True, null=True),
),
]and the migration seems correct. I am adding a new column which is backwards compatible by allowing NULL and blank values. But when running ./manage.py lintmigrations I get this error:
➜ ./manage.py lintmigrations
(axes, 0001_initial)... OK (cached)
(axes, 0002_auto_20151217_2044)... ERR (cached)
NOT NULL constraint on columns
(axes, 0003_auto_20160322_0929)... ERR (cached)
NOT NULL constraint on columns
(axes, 0004_auto_20181024_1538)... ERR (cached)
NOT NULL constraint on columns
(axes, 0005_remove_accessattempt_trusted)... ERR (cached)
NOT NULL constraint on columns
(axes, 0006_remove_accesslog_trusted)... ERR (cached)
NOT NULL constraint on columns
(db, 0001_initial)... OK (cached)
(main, 0001_initial)... OK (cached)
(main, 0002_blogpost_description)... ERR
NOT NULL constraint on columns
*** Summary:
Valid migrations: 3/9 - erroneous migrations: 6/9 - ignored migrations: 0/9Where it is giving me a NOT NULL constraint. Which should not be true as I want to allow it to be nullable. the new migration is 0002_blogpost_description So I cannot figure out why I am getting this error.
I need similar functionality to the "since this commit" commit ID parameter but since I will be running this within a container I will not have access to the git repository. I can manually determine which migrations are new but once I have the list I cannot use them with the current functionality (aside from running the lint once for each name).
I have a couple ideas on how this could be achieved:
Im curious what your thoughts are on this. Im going to explore using the unapplied-migrations flag for now but in a test environment that configures a mock database this wont work since all migrations will be applied or unapplied.
There's no need to check for .git dir, git diff already fails if not inside a git repository
# git diff --name-only --diff-filter=A whatever
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
On a repo that the django project is not in the root, there's no $django_path/.git dir there and is a proper git repo
Maybe not now, but once we only support python 3.6, we can start to type annotate the linter in a uniform way.
And add mypy as static linter
======================================================================
ERROR: test_latest_migrations (apps.b3_core.tests.test_migrations.MigrationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
...
File "/home/melvyn/.local/py-env/django-ECigcvew/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 102, in lint_all_migrations
self.lint_migration(*m)
File "/home/melvyn/.local/py-env/django-ECigcvew/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 69, in lint_migration
sql_statements)
File "/home/melvyn/.local/py-env/django-ECigcvew/lib/python3.6/site-packages/django_migration_linter/sql_analyser.py", line 72, in analyse_sql_statements
if test['fn'](statement, errors=errors):
File "/home/melvyn/.local/py-env/django-ECigcvew/lib/python3.6/site-packages/django_migration_linter/sql_analyser.py", line 24, in has_default
for err in kwargs['errors']
StopIteration
----------------------------------------------------------------------
Ran 1 test in 10.889
Because next() is called on an array with no elements (none match the if statement in the generator).
We can't call sqlmigrate on migrations that remove unique_together constraints because we get ValueError: Found wrong number (0) of constraints for model(field_ref, field_ref). This because Django has smart logic to find the name of the contraint to drop by inspecting the table, but once this migration has been applied, it cannot find the constraint anymore.
Django migrations need more linting than just backward incompatible checks.
Some ideas/issues to be thought about/created:
It is not quite clear to me what kind of checks are there.
I would like to have a full list of rules that this linter enforces.
Thanks!
When a migration contains a data migration, and custom function calls, raise warnings on this migration.
Add warnings when a RunPython function is called in a migration.
Also warn when no reverse migration is specified (and it would fail to roll it back).
Maybe warn when a no-op is the reverse? 🤔
Warnings should stack with errors.
Warnings should be able to be ignored, like some errors.
Warnings won't raise a error code != 0 (apart if bonus is done ;) )
Bonus: add an option to handle warnings as errors
Linting a lot of migrations can take quite some time, and migrations rarely change.
We can cache the result of linting a migration and consult the cache when
black cache system https://github.com/ambv/black#ignoring-unmodified-files--no-cache option, which ignores the cache and lints everything--cache-path PATH which looks for the cache in this location and creates it there. Overrides the default path{
"hash_1": DATA_1,
"hash_2": DATA_2
...
There are 3 states of migrations: OK, IGNORED, or ERROR. Evaluate if we should cache IGNORED and/or ERROR, and if yes, what information needs to be stored along with the state
For instance, certain operations will potentially lock a table, which can have implications during deployment (lots of operations require the table => we don't acquire the lock waiting to migrate / one we have the lock, long migration, locking the table and making production hang)
https://github.com/yandex/zero-downtime-migrations might be interesting to help out for these warnings
Adding a new column to an existing table seems to trigger an error for the NOT_NULL test even if a default is provided. This is contrary to the docs list of incompatible migrations. Here's example output from sqlmigrate using postgresql and django 1.11:
BEGIN;
--
-- Add field surname to person
--
ALTER TABLE "contacts_person" ADD COLUMN "surname" varchar(100) DEFAULT '' NOT NULL;
ALTER TABLE "contacts_person" ALTER COLUMN "surname" DROP DEFAULT;
COMMIT;
It looks like the NOT_NULL test looks for SET DEFAULT, but it doesn't look for just DEFAULT. As far as I can tell from my searching, SET DEFAULT is sql syntax for altering columns and just DEFAULT is the sql syntax for creating columns and this syntax is consistent between mysql and postgresql.
Hopefully I'm not missing something silly 😅but if this issue seems legitimate, I'm happy to try out making a PR.
(Thanks for open-sourcing this package!)
Decide for a license since without license this project is not usable.
Probably look into choosealicense.
Hi, I am using this project template for my project: https://github.com/wemake-services/wemake-django-template
I have installed django-migration-linter and used it like so: django-migration-linter .
That's how it failed:
» django-migration-linter .
Traceback (most recent call last):
File "/Users/sobolev/Documents/PyCharmProjects/rostelecom_svetochka/.venv/bin/django-migration-linter", line 11, in <module>
sys.exit(_main())
File "/Users/sobolev/Documents/PyCharmProjects/rostelecom_svetochka/.venv/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 339, in _main
no_cache=args.no_cache,
File "/Users/sobolev/Documents/PyCharmProjects/rostelecom_svetochka/.venv/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 70, in __init__
self.old_cache = Cache(self.django_path, self.cache_path)
File "/Users/sobolev/Documents/PyCharmProjects/rostelecom_svetochka/.venv/lib/python3.6/site-packages/django_migration_linter/cache.py", line 26, in __init__
project_name = split_path(django_folder)[-2]
IndexError: list index out of rangeI have also tried to use django-migration-linter server, but is said that server is not a django app.
As a result I was not able to run this linter for my setup.
Do you have any ideas how it can be solved? Thanks!
Caching always loads a part of itself, even when deactivated.
Integrate it a bit better to that no caching errors can occur when using --no-cache
Currently, the tests are mostly integration and we have dozens of "fake" django projects in this lib to simulate more or less the real-world usage. This is not very intuitive.
Investigate and implement a way to have more unit tests. Remove some of the integration tests (only few should be enough)
It would great if there is a build-in support for running the linter only on pending migrations, so CI will fail if any pending migrations are non backward compatible
Especially when starting to adopt this tool. It is tedious work to annotate them all.
And I foresee we'll need to do it again if we decide to perform a squashing of existing migrations.
Write tests, but this will require the fake_database_backends to be extended to postgre
Right now every file is in the root directory. This is not really good.
We should have a proper source folder called src or similar and a folder called tests.
I would suggest following structure
📁 docs
📁 src
📁 tests
📄 LICENSE
📄 README.md
📄 .gitignore
Blocked by #90
^ adds "warnings" as returned values of the linter
Handle the warnings by adding to the options:
Probably it would be good to publish this package on PyPi, so a lot of people can easily access this project and try it out.
We certainly missed some SQL migrations that can be done easily in Django, but we want to detect with the linter.
See https://github.com/3YOURMIND/django-migration-linter/blob/master/docs/incompatibilities.md for the current list of linted SQL operations
Let us drop Python 2 support here and use newer Python 3 features like fstrings.
On the through model a unique constraint is added, which then triggers error. This is a classic case for inspecting the operation before calling sqlmigrate, as you cannot safely determine if the alter table statement that Django issues is for a through table or not or even for an existing table now used as through table.
And add python 2 and 3 tests to Travis
Releases and tags for 1.4.0 and 1.4.1 are missing.
I should add them soon
From the README:
--verbose or -v | Print more information during execution.
However, neither -v nor --verbose arguments to the lintmigrations command don't seem to work. Specifically:
$ python src/manage.py lintmigrations -v
manage.py lintmigrations: error: argument -v/--verbosity: expected one argument$ python src/manage.py lintmigrations --verbose
manage.py lintmigrations: error: unrecognized arguments: --verbosedjango-migration-linter==1.4.0
django==2.2.7
python 3.7.5
Let's say we have the following migration:
operations = [
migrations.DeleteModel(
name='A',
),
]
It's not a backward compatible change so should be catched by linter but it's not.
(app_drop_table, 0001_initial)... OK
(app_drop_table, 0002_delete_a)... OK
The tool cries when you try to add a new table with a NOT NULL. This is not nice, and should be improved.
Eg:
new Table A with a FKey which cannot be NULL --> should be fine
new Table A with a field that cannot be NULL --> should be fine.
Would be really nice to integrate a CI service like CircleCI which would check if our code is still runnable.
We wrote tests already so we can test them there too.
Speed-up the linter by changing the usage from a command line tool to a Django management command.
Basically, instead of doing
django-migration-linter my_project/
you'll do
python my_project/manage.py lintmigrations
This means:
git historyINSTALLED_APPS of your projectmakelintermigrations to override makemigrations with linting)What do you think about this @flixx? I started writing a small Proof of Concept to see if it works.
I tried this library, but found that it didn't work in cases where python3 is a different executable than python. This setup is pretty common when running both python 2 and python 3 together. The line that causes the issue is here I believe
I am not sure how other libraries handle it, but ideally it could detect if it was installed with python 3 or python 2. Currently I get a syntax error on all python3 code.
Linting process fails with an exception when using django_celery_monitor.
I expect it just to ignore apps without migrations.
Traceback:
(django_celery_monitor, 0001_initial)... Traceback (most recent call last):
File "/Users/sobolev/Documents/PyCharmProjects/rostelecom_svetochka/.venv/bin/django-migration-linter", line 11, in <module>
sys.exit(_main())
File "/Users/sobolev/Documents/PyCharmProjects/rostelecom_svetochka/.venv/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 341, in _main
linter.lint_all_migrations(git_commit_id=args.commit_id)
File "/Users/sobolev/Documents/PyCharmProjects/rostelecom_svetochka/.venv/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 145, in lint_all_migrations
self.lint_migration(m)
File "/Users/sobolev/Documents/PyCharmProjects/rostelecom_svetochka/.venv/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 104, in lint_migration
sql_statements = self.get_sql(app_name, migration_name)
File "/Users/sobolev/Documents/PyCharmProjects/rostelecom_svetochka/.venv/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 193, in get_sql
"sqlmigrate command failed {0}".format(err.decode("utf-8"))
RuntimeError: sqlmigrate command failed CommandError: App 'django_celery_monitor' does not have migrations
Version: django-celery-monitor==1.1.2
Usage:
INSTALLED_APPS: Tuple[str, ...] = (
# Default django apps:
'django.contrib.auth',
'django.contrib.contenttypes',
'django.contrib.sessions',
'django.contrib.messages',
'django.contrib.staticfiles',
# django-admin:
'django.contrib.admin',
'django.contrib.admindocs',
# Celery:
'django_celery_monitor',
...
)I deployed a new version with your changes, see https://pypi.python.org/pypi/django-migration-linter/
But it seems like README.md is still read, and hence wrongly displayed by PyPi
Either we try to improve your current method using pypandoc, or we just re-write the readme as .rst (which is correctly interpreted by GitHub btw)
Improve the linter to integrate directly yo the django makemigrations command.
This way only backward compatible migrations can be generated.
Suggestion of presentation:
makemigrations
0003_auto_1923423432 ...
This migration may not be backwards compatible:
[Show linting errors here]
You can now edit the migration manually and check again. How do you want to proceed?
(1) Check again.
(2) Ignore and proceed.
(3) Delete migration and abort.
One way to integrate it, could be adding the linter to the INSTALLED_APPS in the Django settings, and then the manage.py makemigrations would get our custom behaviour.
ping FYI @flixx
I'm trying to rename a field in backward-compatible manner:
class Comment(models.Model):
room = deprecate_field(models.CharField(max_length=32, db_index=True))
room2 = models.CharField(max_length=32, db_index=True, null=True)
It creates the follofing migration:
operations = [
migrations.AddField(
model_name='comment',
name='room2',
field=models.CharField(db_index=True, max_length=32, null=True),
),
migrations.AlterField(
model_name='comment',
name='room',
field=models.CharField(db_index=True, max_length=32, null=True),
),
]
But linting fails:
(comments, 0002_auto_20190717_1208)... ERR
NOT NULL constraint on columns
*** Summary:
Valid migrations: 0/1 - erroneous migrations: 1/1 - ignored migrations: 0/1
Raw SQL created by Django 2.2 for postgresql looks like this:
$ python manage.py sqlmigrate comments 0002_auto_20190717_1208
BEGIN;
--
-- Add field room2 to comment
--
ALTER TABLE "comments_comment" ADD COLUMN "room2" varchar(32) NULL;
--
-- Alter field room on comment
--
ALTER TABLE "comments_comment" ALTER COLUMN "room" DROP NOT NULL;
CREATE INDEX "comments_comment_room2_b53b68fe" ON "comments_comment" ("room2");
CREATE INDEX "comments_comment_room2_b53b68fe_like" ON "comments_comment" ("room2" varchar_pattern_ops);
COMMIT;
So, the related test should be fixed somehow:
https://github.com/3YOURMIND/django-migration-linter/blob/master/django_migration_linter/sql_analyser.py#L39
Pretty straightforward. Have a --version that prints the current version of the linter.
When GIT_COMMIT_ID option is specified, MigrationLinter executes diff command on settings directory with --relative parameter sellected:
git_diff_command = (
"cd {0} && git diff --relative --name-only --diff-filter=AR {1}"
).format(self.django_path, git_commit_id)
If settings directory is not located in the root directory of the project, no migrations fill be found.
Proposal to use current directory, and run python manage.py lintmigrations always from the project root directory.
def handle(self, *args, **options):
>> current_path = os.getcwd()
if options["verbosity"] > 1:
logging.basicConfig(format="%(message)s", level=logging.DEBUG)
else:
logging.basicConfig(format="%(message)s")
linter = MigrationLinter(
>> current_path,
ignore_name_contains=options["ignore_name_contains"],
ignore_name=options["ignore_name"],
include_apps=options["include_apps"],
exclude_apps=options["exclude_apps"],
database=options["database"],
cache_path=options["cache_path"],
no_cache=options["no_cache"],
only_applied_migrations=options["applied_migrations"],
only_unapplied_migrations=options["unapplied_migrations"],
)
Other sollution: not to use --relative option.
When I create a migration which alters a Field (e.g. from CharField to IntegerField), the migration linter let this migration pass, although it is not backward compatible.
For example adding options like --hide-error --hide-ok --hide-ignored
Which would then not show output if any of those states occurred.
I can see the latter two being more useful for development, the second to be more useful for CI and the first one is there for completeness
Our apps live in a subfolder called apps/. When I execute the linter I get this:
Traceback (most recent call last):
File "/var/www/django/apps/b3_core/tests/test_migrations.py", line 39, in test_latest_migrations
linter.lint_all_migrations()
File "/usr/local/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 149, in lint_all_migrations
self.lint_migration(*m)
File "/usr/local/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 79, in lint_migration
md5hash = self.old_cache.md5(app_name, migration_name)
File "/usr/local/lib/python3.6/site-packages/django_migration_linter/cache.py", line 48, in md5
with open(path, "rb") as f:
FileNotFoundError: [Errno 2] No such file or directory: '/var/www/django/voucher/migrations/0001_initial.py'
Probably we would need a better way to get the actual migration file for hashing. Ideas?
Improve documentation by:
doc/ folder by topics (command line options, what is linted, how caching works...)Bonus: reflect on changing the documentation format from rst to markdown
My django project is allocated in a subdirectory of a git repo like: xpbx/xpbx so after c6df7e5 I get this error:
DJANGO_SETTINGS_MODULE=xpbx.test django-migration-linter --no-cache $(pwd)/xpbx origin/2.4.1
(customers, 0010_auto_20181026_1330)... Traceback (most recent call last):
File "/home/vseva/Env/xpbx/bin/django-migration-linter", line 11, in <module>
sys.exit(_main())
File "/home/vseva/Env/xpbx/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 341, in _main
linter.lint_all_migrations(git_commit_id=args.commit_id)
File "/home/vseva/Env/xpbx/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 145, in lint_all_migrations
self.lint_migration(m)
File "/home/vseva/Env/xpbx/lib/python3.6/site-packages/django_migration_linter/migration_linter.py", line 81, in lint_migration
md5hash = self.old_cache.md5(migration.abs_path)
File "/home/vseva/Env/xpbx/lib/python3.6/site-packages/django_migration_linter/cache.py", line 47, in md5
with open(path, "rb") as f:
FileNotFoundError: [Errno 2] No such file or directory: '/home/vseva/Foehn/xpbx/xpbx/xpbx/customers/migrations/0010_auto_20181026_1330.py'
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
Django
The Web framework for perfectionists with deadlines.
Laravel
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
Microsoft
Open source projects and samples from Microsoft.
Google
Google ❤️ Open Source for everyone.
Alibaba
Alibaba Open Source for everyone
D3
Data-Driven Documents codes.
Tencent
China tencent open source team.