sqlmigrate wraps it's output in BEGIN/COMMIT even if atomic transactions are disabled

[Joe Meissler]

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.

[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 use migration.atomic and schema_editor.atomic_migration instead of migration.atomic and connection.features.can_rollback_ddl.

Schema editor instance is created later in MigrationLoader (it isn't available in sqlmigrate's handle()) and its atomic value is based on migration.atomic. I'm not sure when these two flags might have different values. Can you provide an example? How you would like to introduce SchemaEditor in the sqlmigrate?

[Joe Meissler]

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 use migration.atomic and schema_editor.atomic_migration instead of migration.atomic and connection.features.can_rollback_ddl.

Schema editor instance is created later in MigrationLoader (it isn't available in sqlmigrate's handle()) and its atomic value is based on migration.atomic. I'm not sure when these two flags might have different values. Can you provide an example? How you would like to introduce SchemaEditor in the sqlmigrate?

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.

[Mariusz Felisiak]

Replying to Joe Meissler:

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.

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 🤔.

[Joe Meissler]

Replying to Mariusz Felisiak:

Replying to Joe Meissler:

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.

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 🤔.

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.

[Mariusz Felisiak]

Thanks for the discussion 👍