openedx / edx-django-utils

edX utilities for Django Application development.
https://edx.readthedocs.io/projects/edx-django-utils/en/latest/
Apache License 2.0
26 stars 20 forks source link

Support Django 1.10 style middleware #26

Closed mikix closed 5 years ago

mikix commented 5 years ago

In Django 1.10, a new middleware style was introduced, that uses a MIDDLEWARE setting rather than MIDDLEWARE_CLASSES.

In Django 2, only the new style will be supported.

This change allows our middleware to be used both ways, but drops support for Django 1.8.

This package claims to support Django 2, but the current middleware classes can't work with Django 2, right? Was that ever tried?

If Django 1.8 support is important to maintain, I can do an optional import situation and keep support. But I figured the open edx ecosystem is on 1.11+.

Reviewers:

Merge checklist:

Post merge:

mikix commented 5 years ago

I haven't tested yet with a new-style repo. I'm trying to do that with course-discovery now.

But the change should be harmless. We're doing it elsewhere: https://github.com/edx/edx-drf-extensions/commit/84cf1de98338d6a5f11e26158481c2158005151c

But will try to set up a course-discovery PR that points to this branch.

mikix commented 5 years ago

https://github.com/edx/course-discovery/pull/2011

^ That seems to work with this branch. And I also manually tested that still using MIDDLEWARE_CLASSES with this branch continues to work.

mikix commented 5 years ago

Oh I did update the docs in this PR

robrap commented 5 years ago

Oops. Not sure how I missed that. Thanks.