[Django] #22791: makemigrations interactive questioner ignores app labels
Django
-------------------------------+--------------------
Reporter: bendavis78 | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22791>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: bendavis78 | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* needs_better_patch: => 0
* needs_docs: => 0
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:1>
Django
Reporter: bendavis78 | Owner: whoshuu
Type: Cleanup/optimization | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by whoshuu):
* status: new => assigned
* owner: nobody => whoshuu
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:2>
Django
Reporter: bendavis78 | Owner: whoshuu
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by whoshuu):
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:
1. (Original ticket) Interactive questioner will ask a "y/N" question for
an app that was not specified.
2. An outstanding conflict in an unspecified app will throw a
`CommandErorr` if `merge` is set to `False`.
3. An outstanding conflict in an unspecified app **will** create a
migration file if `merge` is set to `True` and `interactive` is left as
`False`.
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:
1. Filter out `conflicts` from `loader.detect_conflicts` in the
`makemigrations` module by `app_labels`. This localizes the required
changes but may be inefficient as conflicts will be scanned for
unspecified apps.
2. Pass in `app_labels` to `loader.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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:3>
Django
Reporter: bendavis78 | Owner: whoshuu
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by whoshuu):
Made a quick change in `makemigrations` using approach 1.
PR sent: https://github.com/django/django/pull/2834
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:4>
Django
Reporter: bendavis78 | Owner: whoshuu
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by whoshuu):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:5>
Django
Reporter: bendavis78 | Owner: whoshuu
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: 0 => 1
Comment:
I left comments for improvement on PR. Please uncheck "Patch needs
improvement" when you update it, thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:6>
Django
Reporter: bendavis78 | Owner: whoshuu
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by whoshuu):
* needs_better_patch: 1 => 0
Comment:
The PR has been updated using suggestions left by Tim.
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:7>
Django
Reporter: bendavis78 | Owner: whoshuu
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Migrations | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* stage: Accepted => Ready for checkin
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:8>
Django
Reporter: bendavis78 | Owner: whoshuu
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Migrations | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bendavis78):
* cc: bendavis78 (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:9>
Django
Reporter: bendavis78 | Owner: whoshuu
Cleanup/optimization | Version: master
Component: Migrations | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"f7a78f9bba4efd0231aec0326a73fdbd004d1faa"]:
{{{
#!CommitTicketReference repository=""
revision="f7a78f9bba4efd0231aec0326a73fdbd004d1faa"
Fixed #22791 -- Invoke interactive questioner only for conflicts in
specified apps.
Thanks bendavis78 for the report and Tim Graham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:10>
Django
Reporter: bendavis78 | Owner: whoshuu
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Migrations | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"6d5238f6c873f8ad652ee206b88849a50e68accc"]:
{{{
#!CommitTicketReference repository=""
revision="6d5238f6c873f8ad652ee206b88849a50e68accc"
[1.7.x] Fixed #22791 -- Invoke interactive questioner only for conflicts
in specified apps.
Thanks bendavis78 for the report and Tim Graham for the review.
Backport of f7a78f9bba from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22791#comment:11>