Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Deprecate WSGIMiddleware #1235

Closed
wants to merge 3 commits into from
Closed

Deprecate WSGIMiddleware #1235

wants to merge 3 commits into from

Conversation

abersheeran
Copy link
Member

Related links: #1049 (comment)

@abersheeran
Copy link
Member Author

It seems that using DeprecationWarning directly will cause the test to fail, how can I fix it?

@abersheeran abersheeran requested review from euri10 and Kludex November 6, 2021 16:12
@euri10
Copy link
Member

euri10 commented Nov 6, 2021

add it in setup.cfg ?

    ignore:The built-in `WSGIMiddleware` will be deprecated in future versions, please use `a2wsgi.WSGIMiddleware` instead.:DeprecationWarning

Kludex
Kludex reviewed Nov 6, 2021
View reviewed changes
Copy link
Member

@Kludex Kludex left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should deprecate the module instead?

Like, we are not strict when it comes to private/public objects, so I guess the middleware and build_environ/WSGIResponder are in the same privacy level. Does it make sense?

@abersheeran
Copy link
Member Author

Maybe we should deprecate the module instead?

Like, we are not strict when it comes to private/public objects, so I guess the middleware and build_environ/WSGIResponder are in the same privacy level. Does it make sense?

Agree with your ideas. But I am not sure how to do it...I have no experience in this area. Is there a module that has already done this for me?

@Kludex
Copy link
Member

Kludex commented Nov 7, 2021

I guess like this: https://github.com/encode/uvicorn/pull/1205/files#diff-7c8a9d71c98f0ed83c32a88a1b7e777ef85776c98869045d24c74e3f7714494a

Yeah, we never deprecated modules on uvicorn before, this would be the first time. We have done it for starlette: https://github.com/encode/starlette/blob/8a3e41a5442239bb2da5f2df6cfaaf4683e49f96/starlette/graphql.py#L12-L17

@Kludex
Copy link
Member

Kludex commented Nov 14, 2021

What's the plan with --interface wsgi?

The idea is to import the middleware from a2wsgi?

@abersheeran
Copy link
Member Author

What's the plan with --interface wsgi?

The idea is to import the middleware from a2wsgi?

Do I need to submit it in this PR? I took a quick look at the code and it shouldn't be difficult to implement.

@Kludex
Copy link
Member

Kludex commented Nov 15, 2021

What's the plan with --interface wsgi?

The idea is to import the middleware from a2wsgi?

Do I need to submit it in this PR? I took a quick look at the code and it shouldn't be difficult to implement.

No, I just want to confirm the best steps.

@Kludex
Copy link
Member

Kludex commented Dec 4, 2021

@abersheeran What's our plan for it? 🤔

@abersheeran
Copy link
Member Author

@abersheeran What's our plan for it? 🤔

I don't quite understand what you mean. 👀

@abersheeran abersheeran mentioned this pull request Jan 1, 2022
Closed
@euri10 euri10 added the wsgi label Feb 10, 2022
@euri10
Copy link
Member

euri10 commented Feb 10, 2022

let's favor #1303

@euri10 euri10 closed this Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kludex Kludex Kludex left review comments

@euri10 euri10 Awaiting requested review from euri10

Assignees
No one assigned
Labels
wsgi
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@abersheeran @euri10 @Kludex