@modify_settings append/remove/prepend is affected by dictionary order

[Manuel Kaufmann]

I found a weird behaviour when using @modify_settings with append and remove at the same time for the same elements. I used this because I needed to modify the order of the MIDDLEWARES classes: inject a middleware before one that it's already defined in that seeing. To do that, it's needed to remove all the middleware after that one (including it) and the re-add them by prepending your own.

Example:

# Original settings
MIDDLEWARES = [
    '1',
    '2',
    '3',
    '4',
    '5',
]

and I need this:

# Expected settings
MIDDLEWARES = [
    '1',
    '2',
    '3',
    'MyOwnMiddlewareHere',
    '4',
    '5',
]

I would use @modify_settings in this way:

@modify_settings(MIDDLEWARES={
    'append': [
        'MyOwnMiddlewareHere',
        '4',
        '5',
    ],
    'remove': [
        '4',
        '5',
    ],
})
class FooBarTest(TestCase):
    ....

This cause different behaviours depending on the Python version we are using:

• Python < 3.6 this will randmly works since it could
  • first remove and then append the items: this will produce the expected result
  • first append and then remove the items: this won't produce the expected result
• Python >= 3.6 this will always produce the enexpected result because 3.6 has "insertion ordering" and append will be applied first all the times
  • we can change this behaviour in 3.6 by putting the remove key first, and it will always produce the expected result.

NOTE: other Python implementation (not CPython) will also experiment this problem depending on their implementation.

I fixed this in my own code by using collections.OrderedDict but I think this should be at least mentioned in the documentation to avoid other people having this issue. Otherwise, it could require an OrderedDict.

I wrote some tests that I'm attaching the patch with some comments and explanations on why this is happening. I'm running the tests like this: tox -e py35,py36 -- tests.settings_tests.tests

NOTE: this affects multiple versions of Django

[Manuel Kaufmann]

Tests to demostrate the inconsisten behaviour of @modify_settings

[Markus Holtermann]

Sounds like a legitimate problem.

[Claude Paroz]

I'm not sure append/removed were designed to run in the same modify_settings and the same setting.
What about documenting to nest modify_settings calls for this use case?

[Tim Graham]

If I understand correctly, the requirement to nest @modify_settings isn't present if targeting Python 3.6+ because dicts are ordered there. I'm not sure what would give an expectation of some order of operations in older versions of Python. I'm not sure it's worth documenting anything about this.

[benjaoming]

Hi!

Sounds like a time-consuming issue for those who might encounter it. I agree with Tim that hopefully someone would think twice about this, but having dealt with issues related to dict key ordering myself, I have to admit that you get it wrong sometimes.

Also, the example in the docs [1] would directly lead to the issue, except that it does append/remove/prepend the same items, but it stills seems like it would easily misguide someone to do that

The solution that Markus and I just discussed was that you can A) use OrderedDict or B) add modify_settings twice like this:

# ...
@modify_settings(MIDDLEWARES={
    'remove': [
        '4',
        '5',
    ],
})
@modify_settings(MIDDLEWARES={
    'append': [
        'MyOwnMiddlewareHere',
        '4',
        '5',
    ],
})

Two suggestions:

1. Updating the (misguiding) docs example [1] so it applies modify_settings several times without elaborating lots about why-
2. Add a warning box about doing append+reject on the same items (re-ordering)

As I understand Tim's comment, the latter would be a bit over the top, but the former could be preferable as it wouldn't complicate the docs much?

[1] ​https://docs.djangoproject.com/en/2.0/topics/testing/tools/#django.test.SimpleTestCase.modify_settings

[Chris Jerdonek]

Another option could simply be to change Django's code to sort before iterating over the dict (perhaps only in older versions of Python). I use this practice in personal projects so that behavior will always be deterministic. Also, I personally don't think relying on dict's implicit ordering should be encouraged. It seems too tricky, and there are other patterns whose behavior is more obvious (e.g. the nesting approach).

[Windson yang]

I think users should know dict is not ordered before ​python3.6. I don't think we should docs this in django

[benjaoming]

@Windson yang
This issue itself is evidence that it's easy to get wrong, and that it will cost a lot of time to understand.. especially if the docs promote a pattern that doesn't work in Python <3.6.

There's a PR now with a note about it, and examples are updated.

[Tim Graham <timograham@…>]

In 08f788b1:

Fixed #29226 -- Doc'd modify_settings() ordering considerations for Python < 3.6.

[Tim Graham <timograham@…>]

In fb9c1f4:

[2.1.x] Fixed #29226 -- Doc'd modify_settings() ordering considerations for Python < 3.6.

Backport of 08f788b169a30d26f50433083aca253a4e4031b2 from master