mirror of
https://github.com/django/django.git
synced 2025-11-07 07:15:35 +00:00
Fixed #36652 -- Increased determinism when loading migrations from disk.
Ordering still depends on pkgutil.iter_modules, which does not guarantee order, but at least now Django is not introducing additional indeterminism, causing CircularDependencyError to appear or not appear in some edge cases. Co-authored-by: Jacob Walls <jacobtylerwalls@gmail.com>
This commit is contained in:
committed by
Jacob Walls
parent
eaf7b563a5
commit
e27cff68a3
@@ -109,11 +109,11 @@ class MigrationLoader:
|
|||||||
if was_loaded:
|
if was_loaded:
|
||||||
reload(module)
|
reload(module)
|
||||||
self.migrated_apps.add(app_config.label)
|
self.migrated_apps.add(app_config.label)
|
||||||
migration_names = {
|
migration_names = [
|
||||||
name
|
name
|
||||||
for _, name, is_pkg in pkgutil.iter_modules(module.__path__)
|
for _, name, is_pkg in pkgutil.iter_modules(module.__path__)
|
||||||
if not is_pkg and name[0] not in "_~"
|
if not is_pkg and name[0] not in "_~"
|
||||||
}
|
]
|
||||||
# Load migrations
|
# Load migrations
|
||||||
for migration_name in migration_names:
|
for migration_name in migration_names:
|
||||||
migration_path = "%s.%s" % (module_name, migration_name)
|
migration_path = "%s.%s" % (module_name, migration_name)
|
||||||
|
|||||||
@@ -1,7 +1,12 @@
|
|||||||
import compileall
|
import compileall
|
||||||
import os
|
import os
|
||||||
|
import subprocess
|
||||||
|
import sys
|
||||||
|
import tempfile
|
||||||
from importlib import import_module
|
from importlib import import_module
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
from django.conf import settings
|
||||||
from django.db import connection, connections
|
from django.db import connection, connections
|
||||||
from django.db.migrations.exceptions import (
|
from django.db.migrations.exceptions import (
|
||||||
AmbiguityError,
|
AmbiguityError,
|
||||||
@@ -649,6 +654,70 @@ class LoaderTests(TestCase):
|
|||||||
test_module.__spec__.origin = module_origin
|
test_module.__spec__.origin = module_origin
|
||||||
test_module.__spec__.has_location = module_has_location
|
test_module.__spec__.has_location = module_has_location
|
||||||
|
|
||||||
|
def test_loading_order_does_not_create_circular_dependency(self):
|
||||||
|
"""
|
||||||
|
Before, for these migrations:
|
||||||
|
app1
|
||||||
|
[ ] 0001_squashed_initial <- replaces app1.0001
|
||||||
|
[ ] 0002_squashed_initial <- replaces app1.0001
|
||||||
|
depends on app1.0001_squashed_initial & app2.0001_squashed_initial
|
||||||
|
app2
|
||||||
|
[ ] 0001_squashed_initial <- replaces app2.0001
|
||||||
|
|
||||||
|
When loading app1's migrations, if 0002_squashed_initial was first:
|
||||||
|
{'0002_squashed_initial', '0001_initial', '0001_squashed_initial'}
|
||||||
|
Then CircularDependencyError was raised, but it's resolvable as:
|
||||||
|
{'0001_initial', '0001_squashed_initial', '0002_squashed_initial'}
|
||||||
|
"""
|
||||||
|
# Create a test settings file to provide to the subprocess.
|
||||||
|
MIGRATION_MODULES = {
|
||||||
|
"app1": "migrations.test_migrations_squashed_replaced_order.app1",
|
||||||
|
"app2": "migrations.test_migrations_squashed_replaced_order.app2",
|
||||||
|
}
|
||||||
|
INSTALLED_APPS = [
|
||||||
|
"migrations.test_migrations_squashed_replaced_order.app1",
|
||||||
|
"migrations.test_migrations_squashed_replaced_order.app2",
|
||||||
|
]
|
||||||
|
tests_dir = Path(__file__).parent.parent
|
||||||
|
with tempfile.NamedTemporaryFile(
|
||||||
|
mode="w", encoding="utf-8", suffix=".py", dir=tests_dir, delete=False
|
||||||
|
) as test_settings:
|
||||||
|
for attr, value in settings._wrapped.__dict__.items():
|
||||||
|
if attr.isupper():
|
||||||
|
test_settings.write(f"{attr} = {value!r}\n")
|
||||||
|
# Provide overrides here, instead of via decorators.
|
||||||
|
test_settings.write(f"DATABASES = {settings.DATABASES}\n")
|
||||||
|
test_settings.write(f"MIGRATION_MODULES = {MIGRATION_MODULES}\n")
|
||||||
|
# Isolate away other test apps.
|
||||||
|
test_settings.write(
|
||||||
|
"INSTALLED_APPS=[a for a in INSTALLED_APPS if a.startswith('django')]\n"
|
||||||
|
)
|
||||||
|
test_settings.write(f"INSTALLED_APPS += {INSTALLED_APPS}\n")
|
||||||
|
test_settings_name = test_settings.name
|
||||||
|
self.addCleanup(os.remove, test_settings_name)
|
||||||
|
|
||||||
|
test_environ = os.environ.copy()
|
||||||
|
test_environ["PYTHONPATH"] = str(tests_dir)
|
||||||
|
# Ensure deterministic failures.
|
||||||
|
test_environ["PYTHONHASHSEED"] = "1"
|
||||||
|
|
||||||
|
args = [
|
||||||
|
sys.executable,
|
||||||
|
"-m",
|
||||||
|
"django",
|
||||||
|
"showmigrations",
|
||||||
|
"app1",
|
||||||
|
"--skip-checks",
|
||||||
|
"--settings",
|
||||||
|
Path(test_settings_name).stem,
|
||||||
|
]
|
||||||
|
try:
|
||||||
|
subprocess.run(
|
||||||
|
args, capture_output=True, env=test_environ, check=True, text=True
|
||||||
|
)
|
||||||
|
except subprocess.CalledProcessError as err:
|
||||||
|
self.fail(err.stderr)
|
||||||
|
|
||||||
|
|
||||||
class PycLoaderTests(MigrationTestBase):
|
class PycLoaderTests(MigrationTestBase):
|
||||||
def test_valid(self):
|
def test_valid(self):
|
||||||
|
|||||||
@@ -0,0 +1,9 @@
|
|||||||
|
from django.db import migrations
|
||||||
|
|
||||||
|
|
||||||
|
class Migration(migrations.Migration):
|
||||||
|
initial = True
|
||||||
|
|
||||||
|
dependencies = []
|
||||||
|
|
||||||
|
operations = []
|
||||||
@@ -0,0 +1,13 @@
|
|||||||
|
from django.db import migrations
|
||||||
|
|
||||||
|
|
||||||
|
class Migration(migrations.Migration):
|
||||||
|
initial = True
|
||||||
|
|
||||||
|
replaces = [
|
||||||
|
("app1", "0001_initial"),
|
||||||
|
]
|
||||||
|
|
||||||
|
dependencies = []
|
||||||
|
|
||||||
|
operations = []
|
||||||
@@ -0,0 +1,16 @@
|
|||||||
|
from django.db import migrations
|
||||||
|
|
||||||
|
|
||||||
|
class Migration(migrations.Migration):
|
||||||
|
initial = True
|
||||||
|
|
||||||
|
replaces = [
|
||||||
|
("app1", "0001_initial"),
|
||||||
|
]
|
||||||
|
|
||||||
|
dependencies = [
|
||||||
|
("app1", "0001_squashed_initial"),
|
||||||
|
("app2", "0001_squashed_initial"),
|
||||||
|
]
|
||||||
|
|
||||||
|
operations = []
|
||||||
@@ -0,0 +1,11 @@
|
|||||||
|
from django.db import migrations
|
||||||
|
|
||||||
|
|
||||||
|
class Migration(migrations.Migration):
|
||||||
|
initial = True
|
||||||
|
|
||||||
|
dependencies = [
|
||||||
|
("app1", "0001_initial"),
|
||||||
|
]
|
||||||
|
|
||||||
|
operations = []
|
||||||
@@ -0,0 +1,13 @@
|
|||||||
|
from django.db import migrations
|
||||||
|
|
||||||
|
|
||||||
|
class Migration(migrations.Migration):
|
||||||
|
initial = True
|
||||||
|
|
||||||
|
replaces = [
|
||||||
|
("app2", "0001_initial"),
|
||||||
|
]
|
||||||
|
|
||||||
|
dependencies = []
|
||||||
|
|
||||||
|
operations = []
|
||||||
Reference in New Issue
Block a user