Opened 8 months ago
Closed 7 months ago
#36274 closed New feature (fixed)
MigrationWriter does not support writing run_before
| Reported by: | Mikuláš Poul | Owned by: | Mikuláš Poul |
|---|---|---|---|
| Component: | Migrations | Version: | 5.1 |
| Severity: | Normal | Keywords: | migrations migrationswriter run_before |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The MigrationWriter class implements writing a instance of django.db.migrations.migration.Migration. It supports serializing all the options it could have except for run_before. I recently made a PR for a third-party package on remaking migrations (a form of squashing), and had to subclass the writer to be able to include run_before. I think MigrationWriter should support all options available on Migration, to make working with migrations easier.
Change History (7)
comment:1 by , 8 months ago
| Easy pickings: | unset |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
| Type: | Bug → New feature |
comment:2 by , 8 months ago
That is very fair, I guess it is more a feature request then a bug! I also see that atomic is not supported in the writer, would you like me to add the support for atomic while I am there?
comment:3 by , 8 months ago
I also see that
atomicis not supported in the writer, would you like me to add the support foratomicwhile I am there?
If we're going to add support for run_before, even if unused by the framework itself for now, I think it might be worth making an audit of all documented Migration attributes and adding for them yes.
comment:4 by , 8 months ago
I've updated my PR to include atomic support as well. After adding run_before and atomic that should be all of the properties defined on Migration (going by https://github.com/django/django/blob/cd03e8e2d698e9ccefae2f7687a0400329e6cbe6/django/db/migrations/migration.py#L27-L53)
comment:5 by , 8 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:6 by , 7 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
I'm not sure I would qualify it as a bug given the framework itself doesn't make use of this option internally when creating migrations so it had little use in adding support for it.
It's a small non-invasive feature request though, as patch demonstrates, so I don't think it warrants a larger discussion on the forum.