#22791 closed Cleanup/optimization (fixed)
makemigrations interactive questioner ignores app labels
Reported by: | Ben Davis | Owned by: | Huu Nguyen |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Ben Davis | 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
When running makemigrations <app_label>
, the interactive questioner will still ask questions regarding apps you didn't pass to the command, even though the migrations aren't created for those apps.
Change History (11)
comment:1 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 10 years ago
I'd actually change this ticket to, "makemigrations ignores app labels for conflicts". From the looks of it, there are three issues I would classify as improper behavior:
- (Original ticket) Interactive questioner will ask a "y/N" question for an app that was not specified.
- An outstanding conflict in an unspecified app will throw a
CommandErorr
ifmerge
is set toFalse
. - An outstanding conflict in an unspecified app will create a migration file if
merge
is set toTrue
andinteractive
is set toTrue
, and "y" is answered.
The expected behavior is to ignore apps that have not been specified when at least one app has been specified. There are two ways of going about eliminating these behaviors:
- Filter out
conflicts
fromloader.detect_conflicts
in themakemigrations
module byapp_labels
. This localizes the required changes but may be inefficient as conflicts will be scanned for unspecified apps. - Pass in
app_labels
toloader.detect_conflicts()
and only scan for conflicts in the apps that have been specified. This is more efficient but touches more code.
I'd like to hear what people have to say about this. If necessary, I can create the tickets for issues 2 and 3 if merging them all into this one is inappropriate.
comment:4 by , 10 years ago
Made a quick change in makemigrations
using approach 1.
PR sent: https://github.com/django/django/pull/2834
comment:5 by , 10 years ago
Has patch: | set |
---|
comment:6 by , 10 years ago
Patch needs improvement: | set |
---|
I left comments for improvement on PR. Please uncheck "Patch needs improvement" when you update it, thanks.
comment:7 by , 10 years ago
Patch needs improvement: | unset |
---|
The PR has been updated using suggestions left by Tim.
comment:8 by , 10 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Looks good to me. Bumping to RFC to give Andrew a chance to look and ensure he's okay with the approach rather than the other option you mentioned.
comment:9 by , 10 years ago
Cc: | added |
---|
comment:10 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Hi,
Indeed, I've managed to reproduce the behavior you describe.
I agree that we should try and make the interactive a bit smarter if we can (ie, not ask questions that won't be used for creating migrations).
Thanks.