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 _types module by asgiref.typing #1044

Merged
merged 4 commits into from
May 27, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def get_packages(package):
env_marker_below_38 = "python_version < '3.8'"

minimal_requirements = [
"asgiref>=3.3.4",
"click>=7.*",
"h11>=0.8",
"typing-extensions;" + env_marker_below_38,
Expand Down
67 changes: 0 additions & 67 deletions uvicorn/_types.py

This file was deleted.

9 changes: 7 additions & 2 deletions uvicorn/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,12 @@
import socket
import ssl
import sys
from typing import List, Tuple
from typing import List, Tuple, Union

if sys.version_info < (3, 8):
from typing_extensions import Literal
else:
from typing import Literal

import click

Expand Down Expand Up @@ -219,7 +224,7 @@ def __init__(
self.forwarded_allow_ips = forwarded_allow_ips

@property
def asgi_version(self) -> str:
def asgi_version(self) -> Union[Literal["2.0"], Literal["3.0"]]:
return {"asgi2": "2.0", "asgi3": "3.0", "wsgi": "3.0"}[self.interface]

@property
Expand Down
24 changes: 21 additions & 3 deletions uvicorn/lifespan/on.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,27 @@
import asyncio
import logging
from asyncio import Queue
from typing import Union

from asgiref.typing import (
LifespanScope,
LifespanShutdownCompleteEvent,
LifespanShutdownEvent,
LifespanShutdownFailedEvent,
LifespanStartupCompleteEvent,
LifespanStartupEvent,
LifespanStartupFailedEvent,
)

from uvicorn import Config
from uvicorn._types import LifespanReceiveMessage, LifespanScope, LifespanSendMessage

LifespanReceiveMessage = Union[LifespanStartupEvent, LifespanShutdownEvent]
LifespanSendMessage = Union[
LifespanStartupFailedEvent,
LifespanShutdownFailedEvent,
LifespanStartupCompleteEvent,
LifespanShutdownCompleteEvent,
]

STATE_TRANSITION_ERROR = "Got invalid state transition on lifespan protocol."

Expand Down Expand Up @@ -31,7 +49,7 @@ async def startup(self) -> None:
# Keep a hard reference to prevent garbage collection
# See https://github.com/encode/uvicorn/pull/972

await self.receive_queue.put({"type": "lifespan.startup"})
await self.receive_queue.put(LifespanStartupEvent(type="lifespan.startup"))
Copy link
Member Author

Choose a reason for hiding this comment

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

Not doing this gives me:

uvicorn/lifespan/on.py:52: error: Type of TypedDict is ambiguous, could be any of ("LifespanStartupEvent", "LifespanShutdownEvent")
uvicorn/lifespan/on.py:52: error: Argument 1 to "put" of "Queue" has incompatible type "Dict[str, str]"; expected "Union[LifespanStartupEvent, LifespanShutdownEvent]"

If you have suggestions, I'm open to change it 😗

Copy link
Member

Choose a reason for hiding this comment

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

I think if you do something like

lste : LifespanStartupEvent = {"type": "lifespan.startup"}
await self.receive_queue.put(lste)

it should be ok, though I have not tested it

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it should work. But I don't see any real improvement. Is there any?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it should work. But I don't see any real improvement. Is there any?

well not having a mypy error is one in my mind, I'm no mypy pro but found the TypedDict experience quite frustrating and solved most of my issues by doing that, I'm not married to it though so if you have a better idea I'm all ears.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, the idea would be the current implementation on the PR, but it's the same to me. I've never done what I propose on this PR tho, I always do what you suggest, so I'm going to change the implementation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

await self.startup_event.wait()

if self.startup_failed or (self.error_occured and self.config.lifespan == "on"):
Expand All @@ -44,7 +62,7 @@ async def shutdown(self) -> None:
if self.error_occured:
return
self.logger.info("Waiting for application shutdown.")
await self.receive_queue.put({"type": "lifespan.shutdown"})
await self.receive_queue.put(LifespanShutdownEvent(type="lifespan.shutdown"))
await self.shutdown_event.wait()

if self.shutdown_failed or (
Expand Down
2 changes: 1 addition & 1 deletion uvicorn/protocols/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import urllib
from typing import Optional, Tuple

from uvicorn._types import WWWScope
from asgiref.typing import WWWScope


def get_remote_addr(transport: asyncio.Transport) -> Optional[Tuple[str, int]]:
Expand Down