Fixed #27858 -- Prevented read-only commands from creating the django_migrations table. #8082
Conversation
intgr
force-pushed
the
migration-readonly
branch
8 times, most recently
from
baa8789
to
ec21bac
Compare
tests/admin_scripts/tests.py
Outdated
| @@ -1340,15 +1338,9 @@ def test_no_database(self): | |||
|
|
|||
| def test_readonly_database(self): | |||
| """ | |||
| Ensure runserver.check_migrations doesn't choke when a database is read-only | |||
| (with possibly no django_migrations table). | |||
| Ensure runserver.check_migrations doesn't choke when a database is read-only. | |||
There was a problem hiding this comment.
Is this still simluating a read-only database if the mocking is removed? Chop "Ensure" per https://code.djangoproject.com/ticket/27392.
docs/releases/2.0.txt
Outdated
| @@ -163,7 +163,9 @@ Management Commands | |||
| Migrations | |||
| ~~~~~~~~~~ | |||
|
|
|||
| * ... | |||
| * Commands that access migrations in a read-only manner, including | |||
There was a problem hiding this comment.
I don't think this is a noteworthy feature that merits a mention in the release notes.
django/db/migrations/recorder.py
Outdated
| @@ -39,13 +39,19 @@ def __init__(self, connection): | |||
| def migration_qs(self): | |||
| return self.Migration.objects.using(self.connection.alias) | |||
|
|
|||
| def has_table(self): | |||
| """ | |||
There was a problem hiding this comment.
"""Return True if the django_migrations table exists."""
django/db/migrations/executor.py
Outdated
| @@ -86,6 +86,11 @@ def migrate(self, targets, plan=None, state=None, fake=False, fake_initial=False | |||
| Django first needs to create all project states before a migration is | |||
| (un)applied and in a second step run all the database operations. | |||
| """ | |||
|
|
|||
django/db/migrations/executor.py
Outdated
| @@ -86,6 +86,11 @@ def migrate(self, targets, plan=None, state=None, fake=False, fake_initial=False | |||
| Django first needs to create all project states before a migration is | |||
| (un)applied and in a second step run all the database operations. | |||
| """ | |||
|
|
|||
| # Make sure the migrations table is there. We don't want to fail with | |||
There was a problem hiding this comment.
suggested wording :# The django_migrations table must be present to record applied migrations.
django/db/migrations/recorder.py
Outdated
| self.ensure_schema() | ||
| return set(tuple(x) for x in self.migration_qs.values_list("app", "name")) | ||
| if self.has_table(): | ||
| return set(tuple(x) for x in self.migration_qs.values_list("app", "name")) |
There was a problem hiding this comment.
While here, might as well switch this to use single quotes per our coding style.
tests/admin_scripts/tests.py
Outdated
| @@ -1340,15 +1338,9 @@ def test_no_database(self): | |||
|
|
|||
| def test_readonly_database(self): | |||
| """ | |||
| Ensure runserver.check_migrations doesn't choke when a database is read-only | |||
| (with possibly no django_migrations table). | |||
| Ensure runserver.check_migrations doesn't choke when a database is read-only. | |||
There was a problem hiding this comment.
Chop "Ensure" per https://code.djangoproject.com/ticket/27392.
tests/admin_scripts/tests.py
Outdated
| self.cmd.check_migrations() | ||
| # Check a warning is emitted | ||
| self.assertIn("Not checking migrations", self.output.getvalue()) | ||
| self.cmd.check_migrations() |
There was a problem hiding this comment.
Is this still simulating a read-only database if the mocking is removed? This test might be obsolete considering the exception catching in check_migrations() is removed. Some low level tests for the modified MigrationRecorder methods might be in order instead.
tests/migrations/test_commands.py
Outdated
| MigrationRecorder, 'ensure_schema', | ||
| autospec=True, side_effect=patched_ensure_schema) as ensure_schema: | ||
| with mock.patch.object(MigrationRecorder, 'has_table', autospec=True, | ||
| side_effect=patched_has_table) as has_table: |
There was a problem hiding this comment.
I think the old indentation style was fine -- it allows for longer lines.
timgraham
changed the title
intgr
force-pushed
the
migration-readonly
branch
from
ec21bac
to
6604abe
Compare
timgraham
force-pushed
the
migration-readonly
branch
from
6604abe
to
22423bf
Compare
… the django_migrations table. MigrationRecorder now assumes that if the django_migrations table doesn't exist, then no migrations are applied. Reverted documentation change from refs #23808.
timgraham
force-pushed
the
migration-readonly
branch
from
22423bf
to
fda55c7
Compare
MigrationRecorder is changed so that for read-only operations, if the
django_migrations table doesn't exist, it's assumed that no migrations
have been applied, instead of trying to create it.
Reverted documentation change from refs #23808.