[Django] #23628: AlterModelOptions should check for changes

25 views
Skip to first unread message

Django

unread,
Oct 9, 2014, 2:19:55 PM10/9/14
to django-...@googlegroups.com
#23628: AlterModelOptions should check for changes
----------------------------+--------------------
Reporter: Naddiseo | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------
Consider the following model:

{{{
#!python
PERM_DEFS = {
('perm1', 'Desc1') : 123,
('perm2', 'Desc2') : 321
}
class Spam(models.Model):
class Meta:
permissions = list(PERM_DEFS.keys())

}}}

Currently, running `makemigrations` will always generate a new migration
even though the permissions list doesn't change. Perhaps the inspector
could do a `set(old_attr) == set(new_attr)` type operation on list style
attributes such as `permissions` and `default_permissions`.

Current workaround is: `permissions = list(sorted(PERM_DEFS.keys()))`

--
Ticket URL: <https://code.djangoproject.com/ticket/23628>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Oct 11, 2014, 10:39:16 AM10/11/14
to django-...@googlegroups.com
#23628: AlterModelOptions should check for changes
----------------------------+--------------------------------------

Reporter: Naddiseo | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
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_tests: => 0
* needs_docs: => 0


Comment:

Hi,

This actually depends on your Python version.
On Python 3.3 and later, the order of dictionaries will change between
different runs (that feature is called "hash randomization" if you want to
look deeper).

On earlier versions of Python though, that order is consistent so the
autodetector doesn't see any changes and no new migrations are created.

I'm not sure if this warrants the additional code complexity.
If you need a consistent order here you could use an `OrderedDict` or user
`permissions = sorted(PERM_DEFS.keys())`.

--
Ticket URL: <https://code.djangoproject.com/ticket/23628#comment:1>

Django

unread,
Oct 13, 2014, 1:04:43 PM10/13/14
to django-...@googlegroups.com
#23628: AlterModelOptions should check for changes
----------------------------+--------------------------------------

Reporter: Naddiseo | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------------------------

Comment (by Naddiseo):

I don't think there's that much extra complexity: one possible solution:
https://github.com/django/django/pull/3356

--
Ticket URL: <https://code.djangoproject.com/ticket/23628#comment:2>

Django

unread,
Oct 17, 2014, 9:22:08 AM10/17/14
to django-...@googlegroups.com
#23628: AlterModelOptions should check for changes
----------------------------+--------------------------------------
Reporter: Naddiseo | Owner: nobody
Type: Bug | Status: closed
Component: Migrations | Version: 1.7
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
----------------------------+--------------------------------------
Changes (by timgraham):

* status: new => closed
* resolution: => wontfix


Comment:

I don't think there's a good reason to say the ordering of permissions
isn't important. For example, you might have a migration that indexes the
permissions tuple to get a particular permission. Using `sorted()` as
Baptiste suggested seems like the best solution to me.

--
Ticket URL: <https://code.djangoproject.com/ticket/23628#comment:3>

Reply all
Reply to author
Forward
0 new messages