Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support new Django 1.10 middleware API #127

Closed
twschiller opened this issue Aug 28, 2016 · 13 comments
Closed

Support new Django 1.10 middleware API #127

twschiller opened this issue Aug 28, 2016 · 13 comments

Comments

@twschiller
Copy link

Django 1.10 introduced a new middleware API: https://docs.djangoproject.com/en/1.10/topics/http/middleware/

MIDDLEWARE_CLASSES is deprecated in favor of MIDDLEWARE.

Instructions for upgrading the middleware are at:
https://docs.djangoproject.com/en/1.10/topics/http/middleware/#upgrading-pre-django-1-10-style-middleware

The Travis CI configuration is already testing vs. Django 1.10. However, the Django example needs to be updated to use the new api.

@ezarowny
Copy link
Contributor

We're already working on this and I'm hoping to have it out early this week.

@ezarowny
Copy link
Contributor

ezarowny commented Sep 7, 2016

Sorry for the delay! It's still coming.

@twschiller
Copy link
Author

twschiller commented Sep 8, 2016

Thanks, there's no rush since Django 1.10 still supports the old API and many other middlewares haven't updated yet.

I don't have a deep understanding of Django's error model. However, there's a couple complications I see in the new API:

  • Rollbar probably has to be the first MIDDLEWARE because other middlewares can short-circuit: "If a middleware short-circuits, only that middleware and the ones before it in MIDDLEWARE will see the response."
  • There's no obvious way of distinguishing whether an HTTP response is due to an exception raised in middleware or is an expected response: "Exceptions raised from a middleware are converted to the appropriate HTTP response and then passed to the next middleware."

@ezarowny
Copy link
Contributor

I'm thinking we'll have to provide two middleware classes going forwards. Django provides a nice mixin to basically backport old middleware to the new style but you have to be on Django 1.10 to actually use it.

@twschiller
Copy link
Author

twschiller commented Sep 15, 2016

Right, I've seen other projects adopt the mixin. I'm not sure it addresses the two issues I raised in my previous comment, though. As an alternative to having two middleware classes, you might be able to do one of the following with a little Python hackery:

  • Dynamically mixin the class, if the mixin is available (because the user has Django 1.10 installed)
  • Create a dummy mixin that provides nothing, and choose between the dummy mixin and the backport middleware dynamically

@ezarowny
Copy link
Contributor

That's basically what I was getting at. We'll probably do some kind of dynamic mixin.

@ghost
Copy link

ghost commented Oct 19, 2016

Is there a workaround for this in the meantime? We're currently blocked as we have some middleware requiring the new API.

@ezarowny
Copy link
Contributor

The workaround would be to keep the Rollbar middleware in MIDDLEWARE_CLASSES until the middleware is updated to be dynamic.

I'm going to try to get to this near the end of this week or the beginning of next.

@ghost
Copy link

ghost commented Oct 19, 2016

@ezarowny from what I can tell Django ignores MIDLEWARE_CLASSES if you have things in MIDDLEWARE so that doesn't work for us.

@ezarowny
Copy link
Contributor

PR up at #138

@ezarowny
Copy link
Contributor

0.13.7 has been released on PyPI. Let me know how that works for you guys.

@wli
Copy link

wli commented Oct 27, 2016

@ezarowny FYI I think there might be a regression: #140

@ezarowny
Copy link
Contributor

Yup! Testing #141 now.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants