🚨 Cover middleware/wsgi.py on mypy #1075

Vibhu-Agarwal · 2021-06-09T19:21:55Z

Part of #998
Extends #1018

I've tried to solve most of the issues, but some still remain - not sure how to solve them.

I don't know how to solve the remaining ones

Vibhu-Agarwal · 2021-06-09T19:43:21Z

Not sure if this commit ^^^ is correct.
Will revert, if it's not.

Vibhu-Agarwal · 2021-06-09T19:45:31Z

uvicorn/middleware/wsgi.py

+        message: HTTPRequestEvent = await receive()
        body = message.get("body", b"")


Before the change, it was throwing this error:

uvicorn/middleware/wsgi.py:103: error: TypedDict "HTTPDisconnectEvent" has no key 'body' [misc] uvicorn/middleware/wsgi.py:103: error: TypedDict "WebsocketConnectEvent" has no key 'body' [misc] uvicorn/middleware/wsgi.py:103: error: TypedDict "WebsocketReceiveEvent" has no key 'body' [misc] uvicorn/middleware/wsgi.py:103: error: TypedDict "WebsocketDisconnectEvent" has no key 'body' [misc] uvicorn/middleware/wsgi.py:103: error: TypedDict "LifespanStartupEvent" has no key 'body' [misc] uvicorn/middleware/wsgi.py:103: error: TypedDict "LifespanShutdownEvent" has no key 'body' [misc]

The http.disconnect never gets here?

That's exactly what was bugging me 😅
Most probably http.disconnect could get here.
But if I remove the type annotation from message, the body = message.get("body", b"") line throws out the errors.
Not sure how to resolve that then.

PS. The same goes for body_message a couple of lines below that.

Kludex

Thanks for the PR @Vibhu-Agarwal ! 😃

I've added some comments, can you check them, please?

uvicorn/middleware/wsgi.py

Kludex · 2021-06-09T19:58:32Z

uvicorn/middleware/wsgi.py

+        message: HTTPRequestEvent = await receive()
        body = message.get("body", b"")


The http.disconnect never gets here?

uvicorn/middleware/wsgi.py

euri10 · 2021-06-10T13:20:23Z

I've not looked at this but you can maybe take inspiration from #1049 where I typed everything in the same module I think

Taking ideas from: https://github.com/encode/uvicorn/blob/dd85cdacf154529ea1c3d12b5bda7808673979f2/uvicorn/middleware/wsgi.py

euri10 · 2021-06-21T06:59:39Z

uvicorn/middleware/wsgi.py

-            self.send_queue.append(
-                {"type": "http.response.body", "body": chunk, "more_body": True}
-            )
+    def wsgi(self, environ: Dict, start_response: Awaitable[None]) -> None:


Once #1067 is merged (which is soon !) there will be a _types.py you can use for typing this

Kludex

We're almost there! I've added some comments, do you mind taking a look @Vibhu-Agarwal ?

Thanks for your help so far!

Kludex · 2021-06-21T21:46:33Z

uvicorn/middleware/wsgi.py

        if corrected_name in environ:
-            value = environ[corrected_name] + "," + value
-        environ[corrected_name] = value
+            corrected_name_environ = environ[corrected_name]


This looks a little too much just to satisfy mypy 🤔

Should we just ignore it here?

Kludex · 2021-06-21T21:54:06Z

uvicorn/middleware/wsgi.py

-            status_code, _ = status.split(" ", 1)
-            status_code = int(status_code)
+            status_code_str, _ = status.split(" ", 1)
+            status_code: int = int(status_code_str)


Why do we need to add the annotation here? 🤔

Ah, I guess we needed it before the renaming.

setup.cfg

euri10 · 2021-06-22T04:57:04Z

uvicorn/middleware/wsgi.py

        if corrected_name in environ:
-            value = environ[corrected_name] + "," + value
-        environ[corrected_name] = value
+            corrected_name_environ = environ[corrected_name]


uvicorn/middleware/wsgi.py

euri10

this lgtm @Vibhu-Agarwal thanks a lot
I let @Kludex finish it :)

edit: oups didnt see the CI was failing, will have to wait >_)

euri10 · 2021-06-23T05:10:43Z

you can use this patch @Vibhu-Agarwal I think this solves the rest

Index: tests/middleware/test_wsgi.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests/middleware/test_wsgi.py b/tests/middleware/test_wsgi.py
--- a/tests/middleware/test_wsgi.py	(revision 7610a04a8be1219b2b48eff20e73ad0049884c45)
+++ b/tests/middleware/test_wsgi.py	(revision 6b84705a15ace9d662a49f15921c54bdfd8287c1)
@@ -3,7 +3,7 @@
 
 import httpx
 import pytest
-from asgiref.typing import HTTPScope
+from asgiref.typing import HTTPRequestEvent, HTTPScope
 
 from uvicorn._types import Environ, StartResponse
 from uvicorn.middleware.wsgi import WSGIMiddleware, build_environ
@@ -16,7 +16,7 @@
         ("Content-Type", "text/plain; charset=utf-8"),
         ("Content-Length", str(len(output))),
     ]
-    start_response(status, headers)
+    start_response(status, headers, None)
     return [output]
 
 
@@ -27,7 +27,7 @@
         ("Content-Type", "text/plain; charset=utf-8"),
         ("Content-Length", str(len(output))),
     ]
-    start_response(status, headers)
+    start_response(status, headers, None)
     return [output]
 
 
@@ -45,7 +45,7 @@
             ("Content-Type", "text/plain; charset=utf-8"),
             ("Content-Length", str(len(output))),
         ]
-        start_response(status, headers, exc_info=sys.exc_info())
+        start_response(status, headers, sys.exc_info())  # type: ignore[arg-type]
         return [output]
 
 
@@ -108,7 +108,12 @@
         "root_path": "/文",
         "query_string": b"a=123&b=456",
         "headers": [(b"key", b"value1"), (b"key", b"value2")],
+    }  # type: ignore[typeddict-item]
+    message: HTTPRequestEvent = {
+        "type": "http.request",
+        "body": b"",
+        "more_body": False,
     }
-    environ = build_environ(scope, b"", b"")
+    environ = build_environ(scope, message, b"")
     assert environ["PATH_INFO"] == "/文".encode("utf8").decode("latin-1")
     assert environ["HTTP_KEY"] == "value1,value2"
Index: uvicorn/_types.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/uvicorn/_types.py b/uvicorn/_types.py
--- a/uvicorn/_types.py	(revision 7610a04a8be1219b2b48eff20e73ad0049884c45)
+++ b/uvicorn/_types.py	(revision 6b84705a15ace9d662a49f15921c54bdfd8287c1)
@@ -9,4 +9,6 @@
 StartResponse = typing.Callable[
     [str, typing.Iterable[typing.Tuple[str, str]], typing.Optional[ExcInfo]], None
 ]
-WSGIApp = typing.Callable[[Environ, StartResponse], typing.Iterable[bytes]]
+WSGIApp = typing.Callable[
+    [Environ, StartResponse], typing.Union[typing.Iterable[bytes], BaseException]
+]

Vibhu-Agarwal · 2021-06-23T07:21:53Z

Failing on 3.6 and 3.7 🤔

euri10 · 2021-06-23T07:24:27Z

typeddict is > 3.7 just need to fix the import

…

On Wed, Jun 23, 2021 at 9:22 AM Vibhu Agarwal ***@***.***> wrote: Failing on 3.7 and 3.8 🤔 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1075 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAINSPSUE25MMWUZBNDTE2LTUGDRXANCNFSM46MVJV5A> .

Vibhu-Agarwal · 2021-06-23T07:56:02Z

Unable to figure out the fix 😕
Import doesn't seem like the issue here, as it's coming directly from asgiref where they're checking the version and aptly importing from typing/typing_extensions.
For some reason, # type: ignore isn't working 🤔

euri10 · 2021-06-23T08:13:48Z

you can fill the scope too

    scope: HTTPScope = {
        "type": "http",
        "asgi": {"version": "3.0", "spec_version": "2.0"},
        "scheme": "http",
        "http_version": "1.1",
        "method": "GET",
        "path": "/文",
        "raw_path": b"/\xe6\x96\x87",
        "root_path": "/文",
        "query_string": b"a=123&b=456",
        "client": None,
        "server": None,
        "headers": [(b"key", b"value1"), (b"key", b"value2")],
        "extensions": {},
    }

Vibhu-Agarwal · 2021-06-23T15:16:13Z

Thanks for the reviews and support 👍
Cc @euri10 @Kludex

euri10 · 2021-06-23T15:34:57Z

thanks a lot @Vibhu-Agarwal !

* add types to wsgi module * Add middleware/wsgi.py to setup.cfg * Smoothen out a few (mypy-related) bugs I don't know how to solve the remaining ones * Small Fix: use HTTPRequestEvent * Apply suggestions from PR review * Remove all mypy issues Taking ideas from: https://github.com/encode/uvicorn/blob/dd85cdacf154529ea1c3d12b5bda7808673979f2/uvicorn/middleware/wsgi.py * Adjusting w.r.t. to #1067 * Adjusting w.r.t. to #1067 * Applied suggestions from PR review * Fixed remaining test_wsgi.py mypy issues * Trying by removing "[typeddict-item]" beside "type: ignore" * Revert previous commit * Final fix ... yaayyy! Co-authored-by: Jaakko Lappalainen <[email protected]>

* add types to wsgi module * Add middleware/wsgi.py to setup.cfg * Smoothen out a few (mypy-related) bugs I don't know how to solve the remaining ones * Small Fix: use HTTPRequestEvent * Apply suggestions from PR review * Remove all mypy issues Taking ideas from: https://github.com/encode/uvicorn/blob/dd85cdacf154529ea1c3d12b5bda7808673979f2/uvicorn/middleware/wsgi.py * Adjusting w.r.t. to encode#1067 * Adjusting w.r.t. to encode#1067 * Applied suggestions from PR review * Fixed remaining test_wsgi.py mypy issues * Trying by removing "[typeddict-item]" beside "type: ignore" * Revert previous commit * Final fix ... yaayyy! Co-authored-by: Jaakko Lappalainen <[email protected]>

jkklapp and others added 5 commits April 15, 2021 19:45

add types to wsgi module

7a82d01

Merge jkklapp:add_typing_to_wsgi_module

441c648

Add middleware/wsgi.py to setup.cfg

b181df3

Smoothen out a few (mypy-related) bugs

a80e6bd

I don't know how to solve the remaining ones

Small Fix: use HTTPRequestEvent

3359e85

Vibhu-Agarwal commented Jun 9, 2021

View reviewed changes

Kludex reviewed Jun 9, 2021

View reviewed changes

Apply suggestions from PR review

13ea0fd

Remove all mypy issues

f8ff765

Taking ideas from: https://github.com/encode/uvicorn/blob/dd85cdacf154529ea1c3d12b5bda7808673979f2/uvicorn/middleware/wsgi.py

Vibhu-Agarwal requested a review from Kludex June 10, 2021 20:07

euri10 reviewed Jun 21, 2021

View reviewed changes

Vibhu-Agarwal added 3 commits June 22, 2021 01:05

Merge branch 'encode:master' into jkl_1018

aaab4bb

Adjusting w.r.t. to encode#1067

910385a

Adjusting w.r.t. to encode#1067

6f2b069

Kludex reviewed Jun 21, 2021

View reviewed changes

euri10 requested changes Jun 22, 2021

View reviewed changes

Applied suggestions from PR review

4d19f04

euri10 approved these changes Jun 22, 2021

View reviewed changes

Fixed remaining test_wsgi.py mypy issues

dc1db63

Vibhu-Agarwal added 2 commits June 23, 2021 13:13

Trying by removing "[typeddict-item]" beside "type: ignore"

68e381a

Revert previous commit

8a6cf03

Final fix ... yaayyy!

aa80a78

euri10 approved these changes Jun 23, 2021

View reviewed changes

euri10 merged commit 467e032 into encode:master Jun 23, 2021

Vibhu-Agarwal deleted the jkl_1018 branch June 23, 2021 16:26

		message: HTTPRequestEvent = await receive()
		body = message.get("body", b"")

Uh oh!

🚨 Cover middleware/wsgi.py on mypy #1075

🚨 Cover middleware/wsgi.py on mypy #1075

Uh oh!

Conversation

Vibhu-Agarwal commented Jun 9, 2021

Uh oh!

Vibhu-Agarwal commented Jun 9, 2021

Uh oh!

Vibhu-Agarwal Jun 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kludex left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

euri10 commented Jun 10, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kludex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

euri10 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

euri10 commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vibhu-Agarwal commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

euri10 commented Jun 23, 2021 via email

Uh oh!

Vibhu-Agarwal commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

euri10 commented Jun 23, 2021

Uh oh!

Vibhu-Agarwal commented Jun 23, 2021

Uh oh!

euri10 commented Jun 23, 2021

Uh oh!

Uh oh!

Vibhu-Agarwal Jun 9, 2021 •

edited

Loading

euri10 left a comment •

edited

Loading

euri10 commented Jun 23, 2021 •

edited

Loading

Vibhu-Agarwal commented Jun 23, 2021 •

edited

Loading

Vibhu-Agarwal commented Jun 23, 2021 •

edited

Loading