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.

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

Replace current WSGIMiddleware implementation by a2wsgi one #1825

Merged
merged 15 commits into from
Jan 16, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,35 @@ async def app(scope, receive, send):
})
```

## The WSGI Support

Uvicorn offer support to WSGI applications throught `WSGIMiddleware` which converts
your WSGI into ASGI application seamlessly just by running it with `--interface wsgi`
Copy link
Member

Choose a reason for hiding this comment

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

Is this the first mention to the WSGIMiddleware on the documentation? I think this is just an implementation detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also cut it and put just the mention in the settings like you said


!!! warning
The present WSGIMiddleware will be deprecated in favour of [a2wsgi](https://github.com/abersheeran/a2wsgi).
If you are using Uvicorn to run WSGI applications, please add it as part of your project requirements.
Copy link
Member

Choose a reason for hiding this comment

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

It will be removed, but this PR already deprecates it.

Suggested change
The present WSGIMiddleware will be deprecated in favour of [a2wsgi](https://github.com/abersheeran/a2wsgi).
If you are using Uvicorn to run WSGI applications, please add it as part of your project requirements.
The present WSGIMiddleware is deprecated in favour of [a2wsgi](https://github.com/abersheeran/a2wsgi).
If you are using Uvicorn to run WSGI applications, please add it as part of your project requirements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, updated the docs


To use the WSGI interface install its dependency:

```shell
pip install a2wsgi
```

Create an application, in `example.py`:

```python
def app(environ, start_response):
start_response('200 OK', [('Content-Type', 'text/plain')])
return [b'Hello World!']
```

Run the server:

```shell
$ uvicorn --interface wsgi example:app
Copy link
Member

Choose a reason for hiding this comment

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

Is the --interface needed, or does uvicorn infers it? I never used WSGI with uvicorn. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It fails to infer it, maybe a bug but it falls into asgi2 case that also uses a function instead of a coroutine in the config code:

...
            elif inspect.isfunction(self.loaded_app):
                use_asgi_3 = asyncio.iscoroutinefunction(self.loaded_app)
...

```

---

## Why ASGI?
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ build==0.9.0
twine==4.0.1

# Testing
a2wsgi==1.6.0
autoflake==1.7.7
black==22.10.0
flake8==3.9.2
Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ filterwarnings=
# Turn warnings that aren't filtered into exceptions
error
ignore: \"watchgod\" is depreciated\, you should switch to watchfiles \(`pip install watchfiles`\)\.:DeprecationWarning
ignore: \"WSGIMiddleware\" is deprecated, you should switch to a2wsgi \(`pip install a2wsgi`\)\.:DeprecationWarning
ignore: 'cgi' is deprecated and slated for removal in Python 3\.13:DeprecationWarning

[coverage:run]
Expand Down
51 changes: 41 additions & 10 deletions tests/middleware/test_wsgi.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,38 @@
import io
import sys
from typing import TYPE_CHECKING, AsyncGenerator, List
from importlib import reload
from typing import TYPE_CHECKING, AsyncGenerator, Awaitable, Callable, List
from unittest import mock

import httpx
import pytest

from uvicorn._types import Environ, StartResponse
from uvicorn.middleware.wsgi import WSGIMiddleware, build_environ
from uvicorn.middleware import wsgi

if TYPE_CHECKING:
from asgiref.typing import HTTPRequestEvent, HTTPScope


def environ_switcher(
async_test: Callable[[], Awaitable[None]]
) -> Callable[[bool], Awaitable[None]]:
from uvicorn.middleware import wsgi

async def test_wrapper(use_a2wsgi: bool) -> None:
if use_a2wsgi:
await async_test()
else:
with mock.patch.dict(sys.modules, {"a2wsgi": None}):
reload(wsgi)

await async_test()

reload(wsgi)

return test_wrapper


Copy link
Member

Choose a reason for hiding this comment

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

Now we do this, and avoid the @environ_switcher magic, and the individual parametrize on each test.

Suggested change
def environ_switcher(
async_test: Callable[[], Awaitable[None]]
) -> Callable[[bool], Awaitable[None]]:
from uvicorn.middleware import wsgi
async def test_wrapper(use_a2wsgi: bool) -> None:
if use_a2wsgi:
await async_test()
else:
with mock.patch.dict(sys.modules, {"a2wsgi": None}):
reload(wsgi)
await async_test()
reload(wsgi)
return test_wrapper
@pytest.fixtures(param=wsgi._WSGIMiddleware, a2wsgi.WSGIMiddleware)
def wsgi_middleware_class(request):
return request.param

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, much better now 😄

def hello_world(environ: Environ, start_response: StartResponse) -> List[bytes]:
status = "200 OK"
output = b"Hello World!\n"
Expand All @@ -34,7 +55,7 @@ def echo_body(environ: Environ, start_response: StartResponse) -> List[bytes]:
return [output]


def raise_exception(environ: Environ, start_response: StartResponse) -> RuntimeError:
def raise_exception(environ: Environ, start_response: StartResponse) -> List[bytes]:
Copy link
Member

Choose a reason for hiding this comment

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

I understand why you did this, but I don't think it's the right type. It should be NoReturn 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the typechecker perspective WSGIMiddleware expects a function that has List[bytes] as return type, if it aways fails into exception is not that important.

This type error started to be more clear because the a2wsgi WSGIMiddleware is less permissive in terms of functions it accepts.

raise RuntimeError("Something went wrong")


Expand All @@ -53,56 +74,66 @@ def return_exc_info(environ: Environ, start_response: StartResponse) -> List[byt


@pytest.mark.anyio
@pytest.mark.parametrize("use_a2wsgi", [True, False])
@environ_switcher
async def test_wsgi_get() -> None:
app = WSGIMiddleware(hello_world)
app = wsgi.WSGIMiddleware(hello_world)
async with httpx.AsyncClient(app=app, base_url="http://testserver") as client:
response = await client.get("/")
assert response.status_code == 200
assert response.text == "Hello World!\n"


@pytest.mark.anyio
@pytest.mark.parametrize("use_a2wsgi", [True, False])
@environ_switcher
async def test_wsgi_post() -> None:
app = WSGIMiddleware(echo_body)
app = wsgi.WSGIMiddleware(echo_body)
async with httpx.AsyncClient(app=app, base_url="http://testserver") as client:
response = await client.post("/", json={"example": 123})
assert response.status_code == 200
assert response.text == '{"example": 123}'


@pytest.mark.anyio
@pytest.mark.parametrize("use_a2wsgi", [True, False])
@environ_switcher
async def test_wsgi_put_more_body() -> None:
async def generate_body() -> AsyncGenerator[bytes, None]:
for _ in range(1024):
yield b"123456789abcdef\n" * 64

app = WSGIMiddleware(echo_body)
app = wsgi.WSGIMiddleware(echo_body)
async with httpx.AsyncClient(app=app, base_url="http://testserver") as client:
response = await client.put("/", content=generate_body())
assert response.status_code == 200
assert response.text == "123456789abcdef\n" * 64 * 1024


@pytest.mark.anyio
@pytest.mark.parametrize("use_a2wsgi", [True, False])
@environ_switcher
async def test_wsgi_exception() -> None:
# Note that we're testing the WSGI app directly here.
# The HTTP protocol implementations would catch this error and return 500.
app = WSGIMiddleware(raise_exception)
app = wsgi.WSGIMiddleware(raise_exception)
async with httpx.AsyncClient(app=app, base_url="http://testserver") as client:
with pytest.raises(RuntimeError):
await client.get("/")


@pytest.mark.anyio
@pytest.mark.parametrize("use_a2wsgi", [True, False])
@environ_switcher
async def test_wsgi_exc_info() -> None:
# Note that we're testing the WSGI app directly here.
# The HTTP protocol implementations would catch this error and return 500.
app = WSGIMiddleware(return_exc_info)
app = wsgi.WSGIMiddleware(return_exc_info)
async with httpx.AsyncClient(app=app, base_url="http://testserver") as client:
with pytest.raises(RuntimeError):
response = await client.get("/")

app = WSGIMiddleware(return_exc_info)
app = wsgi.WSGIMiddleware(return_exc_info)
transport = httpx.ASGITransport(
app=app,
raise_app_exceptions=False,
Expand Down Expand Up @@ -136,6 +167,6 @@ def test_build_environ_encoding() -> None:
"body": b"",
"more_body": False,
}
environ = build_environ(scope, message, io.BytesIO(b""))
environ = wsgi.build_environ(scope, message, io.BytesIO(b""))
assert environ["PATH_INFO"] == "/文".encode("utf8").decode("latin-1")
assert environ["HTTP_KEY"] == "value1,value2"
12 changes: 12 additions & 0 deletions uvicorn/middleware/wsgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import concurrent.futures
import io
import sys
import warnings
from collections import deque
from typing import TYPE_CHECKING, Deque, Iterable, Optional, Tuple

Expand Down Expand Up @@ -75,6 +76,11 @@ def build_environ(

class WSGIMiddleware:
Kludex marked this conversation as resolved.
Show resolved Hide resolved
def __init__(self, app: WSGIApp, workers: int = 10):
warnings.warn(
'"WSGIMiddleware" is deprecated, you should switch '
"to a2wsgi (`pip install a2wsgi`).",
Kludex marked this conversation as resolved.
Show resolved Hide resolved
DeprecationWarning,
)
self.app = app
self.executor = concurrent.futures.ThreadPoolExecutor(max_workers=workers)

Expand Down Expand Up @@ -188,3 +194,9 @@ def wsgi(self, environ: Environ, start_response: StartResponse) -> None:
}
self.send_queue.append(empty_body)
self.loop.call_soon_threadsafe(self.send_event.set)


try:
from a2wsgi import WSGIMiddleware # type: ignore # noqa
except ImportError:
pass
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not the most attractive solution, but it's the best option to deprecate it without touching the config module since for testing purposes we can't reload the config module import table without breaking unrelated tests that share the config module state.

It also keeps backward compatibility with someone importing it directly in code.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
try:
from a2wsgi import WSGIMiddleware # type: ignore # noqa
except ImportError:
pass
try:
from a2wsgi import WSGIMiddleware # type: ignore # noqa
except ModuleNotFound:
WSGIMiddleware = _WSGIMiddleware