#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 [email protected].
To post to this group, send email to [email protected].
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.
