#27054: makemigrations tries to create django_migrations in external database
--------------------------------------+------------------------------------
Reporter: SydneyUniLibrary-Jim | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.10
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by SydneyUniLibrary-Jim):
For each connection: `loader.check_consistent_history` calls
`recorder.MigrationRecorder.applied_migrations` to get the set of
migrations that have already been applied to the database on the other end
of the connection. `MigrationRecorder.applied_migrations` calls
`MigrationRecorder.ensure_schema` so that it can query the table
underlying `MigrationRecorder.Migration`.
Conceptually `MigrationRecorder.applied_migrations` is a read-only
operation with respect to the database, but nevertheless it does attempt
write operations as an implementation side-effect.
I'm inexperienced in Django's code, so I'll lift catch the exception
higher up the call stack if you think that's best.
My thoughts at catching the `MigrationSchemaMissing` exception within
`applied_migrations` was along the following lines.
The existing usages of `applied_migrations` are:
*
`django.db.migrations.executor.MigrationsExecutor.check_replacements(self)`:
doesn't handle `MigrationSchemaMissing`, copes with `applied_migrations`
returning an empty set
* `django.db.migrations.loader.MigrationLoader.build_graph(self)`: this
code explicitly treats no connection as being equivalent to
`applied_migrations` returning an empty set
*
`django.db.migrations.loader.MigrationLoader.check_consistent_history(self,
connection)`: uses the result of `applied_migrations` in a for loop, so
copes with `applied_migrations` returning an empty set
`MigrationSchemaMissing` is only raised in
`MigrationRecorder.ensure_schema`.
`MigrationSchemaMissing` is only handled in
`django.core.management.core.management.base.BaseCommand.check_migrations`,
where it logs a message to stdout and then skips checking the migrations.
However `check_migrations` doesn't directly or indirectly call
`applied_migrations` in the try suite.
`MigrationSchemaMissing` is a subclass of `DatabaseError`, which has too
many usages to consider.
Considering the existing usages above, and possible future usages, my
thinking for catching `MigrationSchemaMissing` inside `applied_migrations`
is:
* not being able to create `django_migrations` is not exceptional. I think
my use-case and `BaseCommand.check_migrations` proves that
* `MigrationSchemaMissing` is indirectly raised in `applied_migrations`
because of the implementation side-effect, not because of the semantics of
`applied_migrations`. I think it is leaking an exception in a non-
exceptional circumstance.
* I don't think a callee would usefully want to distinguish between a
database with no migrations, where empty set returned, from a sitatuation
of the `django_migrations` table failed to be created, where
`MigrationSchemaMissing` is raised.
* requiring a callee to have to consider `MigrationSchemaMissing` being
raise is onerous, when returning an empty set would most likely have the
same effect in the callee's code.
--
Ticket URL: <https://code.djangoproject.com/ticket/27054#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/078.27044cefa31c6197322db64330739b0f%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
