Opened 3 months ago
Closed 3 weeks ago
#35656 closed New feature (fixed)
Make migration Command doesn't allow changing the autodetector
Reported by: | Ahmed Ibrahim | Owned by: | Ahmed Ibrahim |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | migrations, makemigrations, commands |
Cc: | Ahmed Ibrahim | 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
I had a situation where I want to subclass the Autodetector to add more operations that the ones in Django, an operation that does not create a row, but rather, a table (different model)
This line is copied from django/django/core/management/commands/makemigrations.py
# Detect changes changes = autodetector.changes( graph=loader.graph, trim_to_apps=app_labels or None, convert_apps=app_labels or None, migration_name=self.migration_name, )
The suggested approach is to save the autodetector
on the class level, and subclasses can override it to use their custom version.
P.S: I'm not sure if this is considered a feature or an optimization
Change History (10)
comment:1 by , 3 months ago
Needs documentation: | set |
---|
comment:2 by , 3 months ago
Needs documentation: | unset |
---|
follow-up: 4 comment:3 by , 3 months ago
Easy pickings: | unset |
---|---|
Triage Stage: | Unreviewed → Accepted |
Version: | → dev |
follow-up: 6 comment:4 by , 3 months ago
Replying to Claude Paroz:
I also do think having a way to specify a custom autodetector class would be an improvement. However, the autodetector class should be the same in
makemigrations
as the one inmigrate
command, so it might not be trivial to define it so it doesn't require the user to customize the class separately in both commands.
Thank you my friend for accepting the ticket and also stating the difficulty of the ticket, yes it bumped up a bit, I will do my best and mention the pr so that you may have a look if you don't mind
comment:5 by , 3 months ago
Has patch: | set |
---|
comment:7 by , 3 months ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | set |
Type: | Cleanup/optimization → New feature |
Marking as a new feature as we should highlight this change in the docs/release notes for people to use it. It also should have tests
comment:8 by , 4 weeks ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Owner: | changed from | to
Patch needs improvement: | unset |
comment:9 by , 3 weeks ago
Triage Stage: | Accepted → Ready for checkin |
---|
I also do think having a way to specify a custom autodetector class would be an improvement. However, the autodetector class should be the same in
makemigrations
as the one inmigrate
command, so it might not be trivial to define it so it doesn't require the user to customize the class separately in both commands.