Skip to content

Django 1.10 functional middleware compatibility #878

New issue

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

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

Already on GitHub? Sign in to your account

Closed
ryankask opened this issue Sep 16, 2016 · 5 comments
Closed

Django 1.10 functional middleware compatibility #878

ryankask opened this issue Sep 16, 2016 · 5 comments

Comments

@ryankask
Copy link
Contributor

debug_toolbar.apps.is_middlware_class() is used in a check for middleware ordering issues.

It assumes that all middleware in MIDDLEWARE are classes and doesn't account for Django 1.10's functional middleware.

If a functional middleware factory is present, it fails when it hits issubclass() since the first argument isn't a class.

I will try to submit a patch this weekend because I need to clone the project and get the test suite running but a simple solution would be tweak it:

def is_middleware_class(middleware_class, middleware_path):
    try:
        middleware_cls = import_string(middleware_path)
    except ImportError:
        return
    try:
        return issubclass(middleware_cls, middleware_class)
    except TypeError:
        return False

Perhaps there is a more elegant check using the types module but I would need to read about Py2/3 compatibility.

@ryankask
Copy link
Contributor Author

ryankask commented Sep 16, 2016

Submitted PR #879 using a more explicit check.

@Benoss
Copy link

Benoss commented Sep 17, 2016

I think this is already covered in this
#853
The pull request is already merged but no releases since

@matthiask
Copy link
Member

Thanks!

@Benoss The check for middleware ordering is still there, and should be fixed if it fails now.

I'll test it in a project of mine and merge if I don't hit any additional problems.

@matthiask
Copy link
Member

That's the same issue as #874 by the way. Closing this issue as duplicate.

@jhancia
Copy link

jhancia commented Sep 27, 2016

Any chance we could have a new PyPi release to incorporate these changes? Thank you so much!

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

No branches or pull requests

4 participants