Skip to content

Commit

Permalink
Correct type annotations in uvicorn/importer.py
Browse files Browse the repository at this point in the history
#1046
#1067

The relationship between module imports and calls in uvicorn/config.py
was previously unclear. `import_from_string` was annotated as returning
a `ModuleType`, but was being used as if it were returning callables.
Mypy was raising "module not callable" errors, as reported in #1046.

Current use cases of `import_from_string` include:

- `uvicorn.Config.HTTP_PROTOCOLS`, `uvicorn.Config.WS_PROTOCOLS`:
  `Type[asyncio.Protocol]` (these are subclasses of asyncio protocols,
  so they are annotated with `Type[Class]` as explained in the mypy docs
- `uvicorn.Config.LOOP_SETUPS`: `Callable` (each loop is a function)
- `uvicorn.Config().loaded_app`: `ASGIApplication` (should be an ASGI
  application instance, like `Starlette()` or `FastAPI()`)

Ideally, the return type of `import_from_string` would reflect these use
cases, but the complex typing will be difficult to maintain. For easier
maintenance, the return type will be `Any`, and objects returned by
`import_from_string` will be annotated directly.
  • Loading branch information
br3ndonland committed Jun 4, 2021
1 parent 8d2cd6f commit e9e59e9
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 15 deletions.
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ files =
uvicorn/middleware/message_logger.py,
uvicorn/supervisors/basereload.py,
uvicorn/importer.py,
tests/importer/test_importer.py,
uvicorn/protocols/utils.py,
uvicorn/loops,
uvicorn/main.py,
Expand Down
12 changes: 6 additions & 6 deletions tests/importer/test_importer.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,40 +3,40 @@
from uvicorn.importer import ImportFromStringError, import_from_string


def test_invalid_format():
def test_invalid_format() -> None:
with pytest.raises(ImportFromStringError) as exc_info:
import_from_string("example:")
expected = 'Import string "example:" must be in format "<module>:<attribute>".'
assert expected in str(exc_info.value)


def test_invalid_module():
def test_invalid_module() -> None:
with pytest.raises(ImportFromStringError) as exc_info:
import_from_string("module_does_not_exist:myattr")
expected = 'Could not import module "module_does_not_exist".'
assert expected in str(exc_info.value)


def test_invalid_attr():
def test_invalid_attr() -> None:
with pytest.raises(ImportFromStringError) as exc_info:
import_from_string("tempfile:attr_does_not_exist")
expected = 'Attribute "attr_does_not_exist" not found in module "tempfile".'
assert expected in str(exc_info.value)


def test_internal_import_error():
def test_internal_import_error() -> None:
with pytest.raises(ImportError):
import_from_string("tests.importer.raise_import_error:myattr")


def test_valid_import():
def test_valid_import() -> None:
instance = import_from_string("tempfile:TemporaryFile")
from tempfile import TemporaryFile

assert instance == TemporaryFile


def test_no_import_needed():
def test_no_import_needed() -> None:
from tempfile import TemporaryFile

instance = import_from_string(TemporaryFile)
Expand Down
2 changes: 1 addition & 1 deletion uvicorn/_handlers/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ async def handle_http(
connection_lost = loop.create_future()

# Switch the protocol from the stream reader to our own HTTP protocol class.
protocol = config.http_protocol_class(
protocol = config.http_protocol_class( # type: ignore[call-arg, operator]
config=config,
server_state=server_state,
on_connection_lost=lambda: connection_lost.set_result(True),
Expand Down
16 changes: 11 additions & 5 deletions uvicorn/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,12 +316,14 @@ def load(self) -> None:
)

if isinstance(self.http, str):
self.http_protocol_class = import_from_string(HTTP_PROTOCOLS[self.http])
http_protocol_class = import_from_string(HTTP_PROTOCOLS[self.http])
self.http_protocol_class: Type[asyncio.Protocol] = http_protocol_class
else:
self.http_protocol_class = self.http

if isinstance(self.ws, str):
self.ws_protocol_class = import_from_string(WS_PROTOCOLS[self.ws])
ws_protocol_class = import_from_string(WS_PROTOCOLS[self.ws])
self.ws_protocol_class: Optional[Type[asyncio.Protocol]] = ws_protocol_class
else:
self.ws_protocol_class = self.ws

Expand Down Expand Up @@ -374,9 +376,13 @@ def load(self) -> None:
self.loaded = True

def setup_event_loop(self) -> None:
loop_setup = import_from_string(LOOP_SETUPS[self.loop])
if loop_setup is not None:
loop_setup()
loop_setup_str: Optional[str] = LOOP_SETUPS[self.loop]
if loop_setup_str:
loop_setup: Callable = import_from_string(loop_setup_str)
if not inspect.isfunction(loop_setup):
raise ImportFromStringError("Asyncio event loop must be a callable.")
else:
loop_setup()

def bind_socket(self) -> socket.socket:
family = socket.AF_INET
Expand Down
5 changes: 2 additions & 3 deletions uvicorn/importer.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import importlib
from types import ModuleType
from typing import Union
from typing import Any


class ImportFromStringError(Exception):
pass


def import_from_string(import_str: Union[ModuleType, str]) -> ModuleType:
def import_from_string(import_str: Any) -> Any:
if not isinstance(import_str, str):
return import_str

Expand Down

0 comments on commit e9e59e9

Please sign in to comment.