#33744 closed Cleanup/optimization (wontfix)
sqlmigrate wraps it's output in BEGIN/COMMIT even if atomic transactions are disabled
Reported by: | Joe Meissler | Owned by: | nobody |
---|---|---|---|
Component: | Migrations | Version: | 4.0 |
Severity: | Normal | Keywords: | |
Cc: | Simon Charette | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
This is very similar to ticket:30189. With that fix, sqlmigrate
now takes into consideration migration.atomic
and connection.features.can_rollback_ddl
. It does not, however, consider if the schema editor is using an atomic migration.
sqlmigrate
can be updated to use migration.atomic and schema_editor.atomic_migration
instead of migration.atomic and connection.features.can_rollback_ddl
.
I'm happy to work on this once it is approved.
Change History (5)
follow-up: 2 comment:1 by , 2 years ago
Cc: | added |
---|---|
Resolution: | → needsinfo |
Status: | new → closed |
Summary: | sqlmigrate wraps it's outpout in BEGIN/COMMIT even if atomic transactions are disabled → sqlmigrate wraps it's output in BEGIN/COMMIT even if atomic transactions are disabled |
Type: | Bug → Cleanup/optimization |
follow-up: 3 comment:2 by , 2 years ago
Replying to Mariusz Felisiak:
Replying to Joe Meissler:
It does not, however, consider if the schema editor is using an atomic migration.
sqlmigrate
can be updated to usemigration.atomic and schema_editor.atomic_migration
instead ofmigration.atomic and connection.features.can_rollback_ddl
.
Schema editor instance is created later in
MigrationLoader
(it isn't available insqlmigrate
'shandle()
) and itsatomic
value is based onmigration.atomic
. I'm not sure when these two flags might have different values. Can you provide an example? How you would like to introduceSchemaEditor
in thesqlmigrate
?
We can access schema_editor
via with connection.schema_editor() as editor
, similar to what is used in the migrate command. As for the atomic
flag, BaseSchemaEditor
has the atomic_migration attribute. You are correct that in the loader, the schema_editor
is instantiated with migration.atomic
. If, however, the editor itself always sets atomic = False
, it doesn't matter that the migration is atomic.
follow-up: 4 comment:3 by , 2 years ago
Replying to Joe Meissler:
We can access
schema_editor
viawith connection.schema_editor() as editor
, similar to what is used in the migrate command. As for theatomic
flag,BaseSchemaEditor
has the atomic_migration attribute. You are correct that in the loader, theschema_editor
is instantiated withmigration.atomic
. If, however, the editor itself always setsatomic = False
, it doesn't matter that the migration is atomic.
So you have a 3rd party backend that ignores migration.atomic
and always set SchemaEditor.atomic_migration = False
and in the same time can roll back DDL (can_rollback_ddl=True
), that's unusual. I'm not sure it's worth extra code 🤔.
comment:4 by , 2 years ago
Resolution: | needsinfo → fixed |
---|
Replying to Mariusz Felisiak:
Replying to Joe Meissler:
We can access
schema_editor
viawith connection.schema_editor() as editor
, similar to what is used in the migrate command. As for theatomic
flag,BaseSchemaEditor
has the atomic_migration attribute. You are correct that in the loader, theschema_editor
is instantiated withmigration.atomic
. If, however, the editor itself always setsatomic = False
, it doesn't matter that the migration is atomic.
So you have a 3rd party backend that ignores
migration.atomic
and always setSchemaEditor.atomic_migration = False
and in the same time can roll back DDL (can_rollback_ddl=True
), that's unusual. I'm not sure it's worth extra code 🤔.
Fair point. Perhaps the backend should implement its own DatabaseFeatures
with can_rollback_ddl = False
as well. I'll resolve as wontfix
unless there's good reason not to.
Replying to Joe Meissler:
Schema editor instance is created later in
MigrationLoader
(it isn't available insqlmigrate
'shandle()
) and itsatomic
value is based onmigration.atomic
. I'm not sure when these two flags might have different values. Can you provide an example? How you would like to introduceSchemaEditor
in thesqlmigrate
?