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

Type-annotate uvicorn/config.py #1067

Merged
merged 28 commits into from
Jun 21, 2021
Merged

Type-annotate uvicorn/config.py #1067

merged 28 commits into from
Jun 21, 2021

Conversation

br3ndonland
Copy link
Contributor

@br3ndonland br3ndonland commented Jun 2, 2021

Description

This PR will add type annotations and mypy type checking to uvicorn/config.py, the associated tests in tests/test_config.py, and the related code that is impacted by the new type annotations.

Supersedes the changes to uvicorn/config.py in #992, and closes #1046.

Changes

uvicorn/config.py

  • Add changes from PR fix: 3rd mypy issues #992 (0db3bc2): This commit will bring in changes to uvicorn/config.py added by @Kludex in PR fix: 3rd mypy issues #992, updating for the latest master branch.
  • Correct SSL type annotations (eee7fba)
    • certfile is a required positional argument when running SSLContext.load_cert_chain, so annotating as Optional (which allows None) would not be ideal. Path-like objects are acceptable, so after from pathlib import Path, the annotation is certfile: Union[Path, str].
    • if self.is_ssl and self.ssl_certfile will help ensure that the self.ssl_certfile required positional argument is present.
  • Simplify log_config type annotation (b2a1548)
  • Add type annotation for app (12d323c):
    • app is sometimes a string, as described in uvicorn/main.py. In other cases, especially in the tests, app is an ASGI application callable.
    • This commit will add a type annotation for the app argument to address these use cases.
  • Simplify if expression in Config().headers (b188fd1)
  • Allow paths to be used for Config(env_file) (d31fddd)
  • Add asyncio Protocol types to class Config kwargs (9c8e70d): Protocol classes are sometimes used for the app kwarg, such as in tests/test_config.py.
  • Improve use of Literal type for ASGI interfaces (14cb546)
    • This commit will retain the use of literals to define ASGI protocol versions, but improve and correct the use of literal types.
    • As described in the mypy docs, Literal["2.0", "3.0"] is a simpler way to write Union[Literal["2.0"], Literal["3.0"]].
  • Add type annotation to Config kwargs in uvicorn/workers.py (459544d)
  • Add type annotations to tests/test_config.py (c436bba)
  • Type-annotate constants in uvicorn/config.py (8d2cd6f): Helps prevent mypy [has-type] errors ("Cannot determine type of {object}")

uvicorn/importer.py

Correct type annotations in uvicorn/importer.py (e9e59e9)

The relationship between module imports and calls in uvicorn/config.py was previously unclear. In #991, import_from_string was annotated as returning a ModuleType, but the function is used to return callables and other types. Mypy was raising "module not callable" errors, as reported in #1046.

uvicorn/uvicorn/config.py

Lines 317 to 325 in 87da6cf

try:
self.loaded_app = import_from_string(self.app)
except ImportFromStringError as exc:
logger.error("Error loading ASGI app. %s" % exc)
sys.exit(1)
try:
self.loaded_app = self.loaded_app()
except TypeError as exc:

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 of the loops 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.

Related

#990
#991
#992
#998
#1046

This commit will bring in changes to uvicorn/config.py added by @Kludex
in PR #992, updating for the latest master branch.
#992
https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain

- `certfile` is a required positional argument when running
  `SSLContext.load_cert_chain`, so annotating as `Optional` (which
  allows `None`) would not be ideal. Path-like objects are acceptable,
  so after `from pathlib import Path`, the annotation is
  `certfile: Union[Path, str]`.
- `if self.is_ssl and self.ssl_certfile` will help ensure that the
  `self.ssl_certfile` required positional argument is present.
`Dict[str, Any]` can be simplified to `dict`.
@Kludex
Copy link
Sponsor Member

Kludex commented Jun 2, 2021

Yeah, the import_from_string probably needs to be fixed.

Somehow related: #1046

@br3ndonland
Copy link
Contributor Author

br3ndonland commented Jun 2, 2021

Yeah, the import_from_string probably needs to be fixed.

Somehow related: #1046

EDIT: I attempted to add a type alias for the return type of import_from_string, but there were many complex mypy errors and it was difficult to manage. Rather than trying to be specific with the return type of import_from_string, I made the return type Any, and shifted the specific type annotations to the objects returned by calls to import_from_string. Should be easier to maintain.

Mypy and tests are passing on my fork.

@br3ndonland br3ndonland marked this pull request as ready for review June 2, 2021 21:18
#990
#992

app is sometimes a string, as described in uvicorn/main.py. In other
cases, especially in the tests, app is an ASGI application callable.

This commit will add a type annotation for the app argument to address
these use cases.
Protocol classes are sometimes used for the app kwarg, such as in
tests/test_config.py.
https://mypy.readthedocs.io/en/stable/literal_types.html

This commit will retain the use of literals to define ASGI protocol
versions, but improve and correct the use of literal types.

As described in the mypy docs, `Literal["2.0", "3.0"]` is a simpler way
to write `Union[Literal["2.0"], Literal["3.0"]]`.
tests/importer/test_importer.py Outdated Show resolved Hide resolved
tests/test_config.py Outdated Show resolved Hide resolved
uvicorn/config.py Outdated Show resolved Hide resolved
uvicorn/config.py Show resolved Hide resolved
uvicorn/config.py Outdated Show resolved Hide resolved
PR #978 added logic to convert `reload_dirs` from string to list, as
shown in `test_reload_dir_is_set()`, so the annotation on reload_dirs
will be updated to accept a string.
Prevents mypy `[has-type]` errors ("Cannot determine type of {object}")
#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.
@euri10
Copy link
Member

euri10 commented Jun 7, 2021

/rant on
this Approve and run button is really annoying
/rant off

#1067 (comment)

Alternative to `pathlib.Path` introduced in Python 3.6.
@br3ndonland
Copy link
Contributor Author

/rant on
this Approve and run button is really annoying
/rant off

Hopefully almost done 🤞

Copy link
Member

@euri10 euri10 left a comment

Choose a reason for hiding this comment

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

another round of comments, sorry this is a massive PR so I was not able to touch on everything at first sight :)
but we're getting there !

tests/test_config.py Outdated Show resolved Hide resolved
tests/test_config.py Outdated Show resolved Hide resolved
#1067 (comment)
#1067 (comment)

https://github.com/encode/starlette/blob/b6f3578bb2cf6c60e3efe110143409b47f368d36/starlette/config.py#L16
https://github.com/python/cpython/blob/3e1c7167d86a2a928cdcb659094aa10bb5550c4c/Lib/os.py#L737
https://docs.pytest.org/en/latest/reference/reference.html#pytest.FixtureRequest

- Add custom types for exceptions and start response
- Use `MutableMapping` for `os.environ`: matches starlette/config.py.
- Use `pytest.FixtureRequest` for fixture requests. The `param`
  attribute is optional, so mypy requires a check for the attribute
  before indexing into it (`getattr(request, "param")`).
#1067

Fixes `[import]` error: Library stubs not installed for "yaml"
@br3ndonland
Copy link
Contributor Author

another round of comments, sorry this is a massive PR so I was not able to touch on everything at first sight :)
but we're getting there !

Cool, thanks for your reviews! Good feedback.

@@ -166,8 +188,10 @@ def test_ssl_config_combined(tls_certificate_pem_path):
assert config.is_ssl is True


def asgi2_app(scope):
async def asgi(receive, send): # pragma: nocover
def asgi2_app(scope: Scope) -> Callable:
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Suggested change
def asgi2_app(scope: Scope) -> Callable:
def asgi2_app(scope: Scope) -> Callable[[ASGIReceiveCallable, ASGISendCallable], Awaitable[None]]:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we use the asgiref types instead?

def asgi2_app(scope: Scope) -> ASGI2Application:
    async def asgi(
        receive: ASGIReceiveCallable, send: ASGISendCallable
    ) -> None:  # pragma: nocover
        pass

    return asgi  # pragma: nocover

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

That doesn't work because the ASGI2Application is all of it. That part is only the callable that I've annotated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That doesn't work because the ASGI2Application is all of it. That part is only the callable that I've annotated.

I see. This is related to #1067 (comment) - if we don't want Config(app) to accept a callable, we might have to refactor these test methods.

tests/test_config.py Show resolved Hide resolved
uvicorn/config.py Outdated Show resolved Hide resolved
uvicorn/config.py Outdated Show resolved Hide resolved
uvicorn/config.py Outdated Show resolved Hide resolved
uvicorn/importer.py Show resolved Hide resolved
@Kludex
Copy link
Sponsor Member

Kludex commented Jun 9, 2021

Thanks for the PR @br3ndonland :)
I've left some comments, I hope you can take a look at them.

@br3ndonland
Copy link
Contributor Author

Thanks for the PR @br3ndonland :)
I've left some comments, I hope you can take a look at them.

Thank you for your help!

#1067 (comment)
https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain

`certfile` is a required argument for  `SSLContext.load_cert_chain`. As
it is currently, `is_ssl` could return `True` without `certfile`.
@br3ndonland
Copy link
Contributor Author

I forgot to remove the type def in line 325 when merging master, can you update ?

Sure, no problem, I updated and the CI checks should be passing now.

@br3ndonland
Copy link
Contributor Author

Thanks @euri10 and @Kludex for all your help on this PR so far.

I will be AFK for a few days, but will check in on this when I return.

Copy link
Member

@euri10 euri10 left a comment

Choose a reason for hiding this comment

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

ok 2 more comments on my side. I'm sorry this takes so long but it's a huge piece of review, hopefully we get to the end :)

tests/test_config.py Outdated Show resolved Hide resolved
uvicorn/config.py Outdated Show resolved Hide resolved
@euri10
Copy link
Member

euri10 commented Jun 17, 2021

Thanks @euri10 and @Kludex for all your help on this PR so far.

I will be AFK for a few days, but will check in on this when I return.

hopefully you're back @br3ndonland ;) can we try to finish this, I know it took already a while but I feel we're near the end !

@br3ndonland
Copy link
Contributor Author

hopefully you're back @br3ndonland ;) can we try to finish this, I know it took already a while but I feel we're near the end !

I'm back! Yes I think we're almost done. Thanks for sticking with it!

Copy link
Member

@euri10 euri10 left a comment

Choose a reason for hiding this comment

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

As far as I'm concerned I'm happy with it.
@Kludex can you please take another look and clear if necessary the conversation [here] (https://github.com/encode/uvicorn/pull/1067/files#r648495399) (will merge once I'm sure nothing's left unanswered!)
Thanks a lot for your time and patience with us on this @br3ndonland but this was not an easy one !!

@Kludex
Copy link
Sponsor Member

Kludex commented Jun 21, 2021

Only the Awaitable in the app parameter on the Config and then we're ready! 🙌

@br3ndonland
Copy link
Contributor Author

Only the Awaitable in the app parameter on the Config and then we're ready! 🙌

Done (2560310). I was thinking about removing Awaitable. Thanks for pointing it out.

In a future PR, I will also look into removing Callable from the app annotation, as discussed in #1067 (comment).

@br3ndonland
Copy link
Contributor Author

Merged in latest master. Should be a clean PR merge with all CI checks passing.

@euri10 euri10 merged commit c71de72 into encode:master Jun 21, 2021
@euri10
Copy link
Member

euri10 commented Jun 21, 2021

let's roll, thanks a lot @br3ndonland !

@Kludex
Copy link
Sponsor Member

Kludex commented Jun 21, 2021

Thanks @br3ndonland ! :)

Vibhu-Agarwal added a commit to Vibhu-Agarwal/uvicorn that referenced this pull request Jun 21, 2021
Vibhu-Agarwal added a commit to Vibhu-Agarwal/uvicorn that referenced this pull request Jun 21, 2021
euri10 pushed a commit that referenced this pull request Jun 23, 2021
* 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 <jkk.lapp@gmail.com>
@br3ndonland br3ndonland deleted the mypy-config branch October 24, 2021 00:55
Kludex pushed a commit that referenced this pull request Nov 17, 2021
* Add changes from PR #992

This commit will bring in changes to uvicorn/config.py added by @Kludex
in PR #992, updating for the latest master branch.

* Correct SSL type annotations

#992
https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain

- `certfile` is a required positional argument when running
  `SSLContext.load_cert_chain`, so annotating as `Optional` (which
  allows `None`) would not be ideal. Path-like objects are acceptable,
  so after `from pathlib import Path`, the annotation is
  `certfile: Union[Path, str]`.
- `if self.is_ssl and self.ssl_certfile` will help ensure that the
  `self.ssl_certfile` required positional argument is present.

* Simplify log_config type annotation

`Dict[str, Any]` can be simplified to `dict`.

* Add type annotation for app

#990
#992

app is sometimes a string, as described in uvicorn/main.py. In other
cases, especially in the tests, app is an ASGI application callable.

This commit will add a type annotation for the app argument to address
these use cases.

* Simplify if expression in Config().headers

* Allow paths to be used for Config(env_file)

* Add asyncio Protocol types to class Config kwargs

Protocol classes are sometimes used for the app kwarg, such as in
tests/test_config.py.

* Improve use of Literal type for ASGI interfaces

https://mypy.readthedocs.io/en/stable/literal_types.html

This commit will retain the use of literals to define ASGI protocol
versions, but improve and correct the use of literal types.

As described in the mypy docs, `Literal["2.0", "3.0"]` is a simpler way
to write `Union[Literal["2.0"], Literal["3.0"]]`.

* Add type annotation to Config kwargs in workers.py

https://mypy.readthedocs.io/en/stable/type_inference_and_annotations.html

This is just one of those times when mypy needs a little help.

* Add type annotations to test_config.py

PR #978 added logic to convert `reload_dirs` from string to list, as
shown in `test_reload_dir_is_set()`, so the annotation on reload_dirs
will be updated to accept a string.

* Type-annotate constants in uvicorn/config.py

Prevents mypy `[has-type]` errors ("Cannot determine type of {object}")

* Correct type annotations in uvicorn/importer.py

#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.

* Use os.PathLike for paths in uvicorn/config.py

#1067 (comment)

Alternative to `pathlib.Path` introduced in Python 3.6.

* Use more specific types in test_config.py

#1067 (comment)
#1067 (comment)

https://github.com/encode/starlette/blob/b6f3578bb2cf6c60e3efe110143409b47f368d36/starlette/config.py#L16
https://github.com/python/cpython/blob/3e1c7167d86a2a928cdcb659094aa10bb5550c4c/Lib/os.py#L737
https://docs.pytest.org/en/latest/reference/reference.html#pytest.FixtureRequest

- Add custom types for exceptions and start response
- Use `MutableMapping` for `os.environ`: matches starlette/config.py.
- Use `pytest.FixtureRequest` for fixture requests. The `param`
  attribute is optional, so mypy requires a check for the attribute
  before indexing into it (`getattr(request, "param")`).

* Install missing YAML type stubs for mypy

#1067

Fixes `[import]` error: Library stubs not installed for "yaml"

* Add Environ type to test_config.py

#1067 (comment)

* Add Literal type aliases for web server config

#1067 (comment)

* Use suggested casing for Literal type aliases

#1067 (comment)
#1067 (comment)

* Restore test_config.py test_app_factory comment

#1067 (comment)

Incorrectly modified in c436bba.

* Simplify event loop setup in config.py

#1067 (comment)

* Remove old type comment after merging master

#1067 (comment)

* Assert that certfile is present for SSL context

#1067 (comment)
https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain

`certfile` is a required argument for  `SSLContext.load_cert_chain`. As
it is currently, `is_ssl` could return `True` without `certfile`.

* Restore support for Config(loop='none')

#455
e9e59e9
e60ba66
#1067 (comment)

* Move WSGI types to uvicorn/_types.py

#1067 (comment)

* Remove Awaitable from app type annotation

#1067 (comment)

Co-authored-by: euri10 <euri10@users.noreply.github.com>
Kludex pushed a commit that referenced this pull request Nov 17, 2021
* 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 <jkk.lapp@gmail.com>
Kludex pushed a commit to sephioh/uvicorn that referenced this pull request Oct 29, 2022
* Add changes from PR encode#992

This commit will bring in changes to uvicorn/config.py added by @Kludex
in PR encode#992, updating for the latest master branch.

* Correct SSL type annotations

encode#992
https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain

- `certfile` is a required positional argument when running
  `SSLContext.load_cert_chain`, so annotating as `Optional` (which
  allows `None`) would not be ideal. Path-like objects are acceptable,
  so after `from pathlib import Path`, the annotation is
  `certfile: Union[Path, str]`.
- `if self.is_ssl and self.ssl_certfile` will help ensure that the
  `self.ssl_certfile` required positional argument is present.

* Simplify log_config type annotation

`Dict[str, Any]` can be simplified to `dict`.

* Add type annotation for app

encode#990
encode#992

app is sometimes a string, as described in uvicorn/main.py. In other
cases, especially in the tests, app is an ASGI application callable.

This commit will add a type annotation for the app argument to address
these use cases.

* Simplify if expression in Config().headers

* Allow paths to be used for Config(env_file)

* Add asyncio Protocol types to class Config kwargs

Protocol classes are sometimes used for the app kwarg, such as in
tests/test_config.py.

* Improve use of Literal type for ASGI interfaces

https://mypy.readthedocs.io/en/stable/literal_types.html

This commit will retain the use of literals to define ASGI protocol
versions, but improve and correct the use of literal types.

As described in the mypy docs, `Literal["2.0", "3.0"]` is a simpler way
to write `Union[Literal["2.0"], Literal["3.0"]]`.

* Add type annotation to Config kwargs in workers.py

https://mypy.readthedocs.io/en/stable/type_inference_and_annotations.html

This is just one of those times when mypy needs a little help.

* Add type annotations to test_config.py

PR encode#978 added logic to convert `reload_dirs` from string to list, as
shown in `test_reload_dir_is_set()`, so the annotation on reload_dirs
will be updated to accept a string.

* Type-annotate constants in uvicorn/config.py

Prevents mypy `[has-type]` errors ("Cannot determine type of {object}")

* Correct type annotations in uvicorn/importer.py

encode#1046
encode#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 encode#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.

* Use os.PathLike for paths in uvicorn/config.py

encode#1067 (comment)

Alternative to `pathlib.Path` introduced in Python 3.6.

* Use more specific types in test_config.py

encode#1067 (comment)
encode#1067 (comment)

https://github.com/encode/starlette/blob/b6f3578bb2cf6c60e3efe110143409b47f368d36/starlette/config.py#L16
https://github.com/python/cpython/blob/3e1c7167d86a2a928cdcb659094aa10bb5550c4c/Lib/os.py#L737
https://docs.pytest.org/en/latest/reference/reference.html#pytest.FixtureRequest

- Add custom types for exceptions and start response
- Use `MutableMapping` for `os.environ`: matches starlette/config.py.
- Use `pytest.FixtureRequest` for fixture requests. The `param`
  attribute is optional, so mypy requires a check for the attribute
  before indexing into it (`getattr(request, "param")`).

* Install missing YAML type stubs for mypy

encode#1067

Fixes `[import]` error: Library stubs not installed for "yaml"

* Add Environ type to test_config.py

encode#1067 (comment)

* Add Literal type aliases for web server config

encode#1067 (comment)

* Use suggested casing for Literal type aliases

encode#1067 (comment)
encode#1067 (comment)

* Restore test_config.py test_app_factory comment

encode#1067 (comment)

Incorrectly modified in c436bba.

* Simplify event loop setup in config.py

encode#1067 (comment)

* Remove old type comment after merging master

encode#1067 (comment)

* Assert that certfile is present for SSL context

encode#1067 (comment)
https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain

`certfile` is a required argument for  `SSLContext.load_cert_chain`. As
it is currently, `is_ssl` could return `True` without `certfile`.

* Restore support for Config(loop='none')

encode#455
encode/uvicorn@e9e59e9
encode/uvicorn@e60ba66
encode#1067 (comment)

* Move WSGI types to uvicorn/_types.py

encode#1067 (comment)

* Remove Awaitable from app type annotation

encode#1067 (comment)

Co-authored-by: euri10 <euri10@users.noreply.github.com>
Kludex pushed a commit to sephioh/uvicorn that referenced this pull request Oct 29, 2022
* 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 <jkk.lapp@gmail.com>
br3ndonland added a commit to br3ndonland/inboard that referenced this pull request Dec 6, 2022
This commit will enable mypy strict mode, and update code accordingly.

Type annotations are not used at runtime. The standard library `typing`
module includes a `TYPE_CHECKING` constant that is `False` at runtime,
but `True` when conducting static type checking prior to runtime. Type
imports will be included under `if TYPE_CHECKING:` conditions. These
conditions will be ignored when calculating test coverage.
https://docs.python.org/3/library/typing.html

The Python standard library `logging.config` module uses type stubs.
The typeshed types for the `logging.config` module are used solely for
type-checking usage of the `logging.config` module itself. They cannot
be imported and used to type annotate other modules. For this reason,
dict config types will be vendored into a module in the inboard package.
https://github.com/python/typeshed/blob/main/stdlib/logging/config.pyi

The ASGI application in `inboard.app.main_base` will be updated to ASGI3
and type-annotated with `asgiref.typing`. Note that, while Uvicorn uses
`asgiref.typing`, Starlette does not. The type signature expected by the
Starlette/FastAPI `TestClient` therefore does not match
`asgiref.typing.ASGIApplication`. A mypy `type: ignore[arg-type]`
comment will be used to resolve this difference.
https://asgi.readthedocs.io/en/stable/specs/main.html

Also note that, while the `asgiref` package was a runtime dependency of
Uvicorn 0.17.6, it was later removed from Uvicorn's runtime dependencies
in 0.18.0 (encode/uvicorn#1305, encode/uvicorn#1532). However, `asgiref`
is still used to type-annotate Uvicorn, so any downstream projects like
inboard that type-check Uvicorn objects must also install `asgiref`.
Therefore, `asgiref` will be added to inboard's development dependencies
to ensure that type checking continues to work as expected.

A Uvicorn options type will be added to a new inboard types module.
The Uvicorn options type will be a `TypedDict` with fields corresponding
to arguments to `uvicorn.run`. This type can be used to check arguments
passed to `uvicorn.run`, which is how `inboard.start` runs Uvicorn.

Uvicorn 0.17.6 is not fully type-annotated, and Uvicorn does not ship
with a `py.typed` marker file until 0.19.0.

It would be convenient to generate types dynamically with something like
`getattr(uvicorn.run, "__annotations__")` (Python 3.9 or earlier)
or `inspect.get_annotations(uvicorn.run)` (Python 3.10 or later).
https://docs.python.org/3/howto/annotations.html

It could look something like this:

```py
UvicornOptions = TypedDict(  # type: ignore[misc]
    "UvicornOptions",
    inspect.get_annotations(uvicorn.run),
    total=False,
)
```

Note the `type: ignore[misc]` comment. Mypy raises a `misc` error:
`TypedDict() expects a dictionary literal as the second argument`.
Unfortunately, `TypedDict` types are not intended to be generated
dynamically, because they exist for the benefit of static type checking
(python/mypy#3932, python/mypy#4128, python/mypy#13940).

Furthermore, prior to Uvicorn 0.18.0, `uvicorn.run()` didn't enumerate
keyword arguments, but instead accepted `kwargs` and passed them to
`uvicorn.Config.__init__()` (encode/uvicorn#1423). The annotations from
`uvicorn.Config.__init__()` would need to be used instead. Even after
Uvicorn 0.18.0, the signatures of the two functions are not exactly the
same (encode/uvicorn#1545), so it helps to have a static type defined.

There will be some other differences from `uvicorn.run()`:

- The `app` argument to `uvicorn.run()` accepts an un-parametrized
  `Callable` because Uvicorn tests use callables (encode/uvicorn#1067).
  It is not necessary for other packages to accept `Callable`, and it
  would need to be parametrized to pass mypy strict mode anyway.
  For these reasons, `Callable` will not be accepted in this type.
- The `log_config` argument will use the new inboard dict config type
  instead of `dict[str, Any]` for stricter type checking.
cr313 added a commit to cr313/py-project-uvicorn that referenced this pull request Apr 19, 2024
* Add changes from PR #992

This commit will bring in changes to uvicorn/config.py added by @Kludex
in PR #992, updating for the latest master branch.

* Correct SSL type annotations

encode/uvicorn#992
https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain

- `certfile` is a required positional argument when running
  `SSLContext.load_cert_chain`, so annotating as `Optional` (which
  allows `None`) would not be ideal. Path-like objects are acceptable,
  so after `from pathlib import Path`, the annotation is
  `certfile: Union[Path, str]`.
- `if self.is_ssl and self.ssl_certfile` will help ensure that the
  `self.ssl_certfile` required positional argument is present.

* Simplify log_config type annotation

`Dict[str, Any]` can be simplified to `dict`.

* Add type annotation for app

encode/uvicorn#990
encode/uvicorn#992

app is sometimes a string, as described in uvicorn/main.py. In other
cases, especially in the tests, app is an ASGI application callable.

This commit will add a type annotation for the app argument to address
these use cases.

* Simplify if expression in Config().headers

* Allow paths to be used for Config(env_file)

* Add asyncio Protocol types to class Config kwargs

Protocol classes are sometimes used for the app kwarg, such as in
tests/test_config.py.

* Improve use of Literal type for ASGI interfaces

https://mypy.readthedocs.io/en/stable/literal_types.html

This commit will retain the use of literals to define ASGI protocol
versions, but improve and correct the use of literal types.

As described in the mypy docs, `Literal["2.0", "3.0"]` is a simpler way
to write `Union[Literal["2.0"], Literal["3.0"]]`.

* Add type annotation to Config kwargs in workers.py

https://mypy.readthedocs.io/en/stable/type_inference_and_annotations.html

This is just one of those times when mypy needs a little help.

* Add type annotations to test_config.py

PR #978 added logic to convert `reload_dirs` from string to list, as
shown in `test_reload_dir_is_set()`, so the annotation on reload_dirs
will be updated to accept a string.

* Type-annotate constants in uvicorn/config.py

Prevents mypy `[has-type]` errors ("Cannot determine type of {object}")

* Correct type annotations in uvicorn/importer.py

encode/uvicorn#1046
encode/uvicorn#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.

* Use os.PathLike for paths in uvicorn/config.py

encode/uvicorn#1067 (comment)

Alternative to `pathlib.Path` introduced in Python 3.6.

* Use more specific types in test_config.py

encode/uvicorn#1067 (comment)
encode/uvicorn#1067 (comment)

https://github.com/encode/starlette/blob/b6f3578bb2cf6c60e3efe110143409b47f368d36/starlette/config.py#L16
https://github.com/python/cpython/blob/3e1c7167d86a2a928cdcb659094aa10bb5550c4c/Lib/os.py#L737
https://docs.pytest.org/en/latest/reference/reference.html#pytest.FixtureRequest

- Add custom types for exceptions and start response
- Use `MutableMapping` for `os.environ`: matches starlette/config.py.
- Use `pytest.FixtureRequest` for fixture requests. The `param`
  attribute is optional, so mypy requires a check for the attribute
  before indexing into it (`getattr(request, "param")`).

* Install missing YAML type stubs for mypy

encode/uvicorn#1067

Fixes `[import]` error: Library stubs not installed for "yaml"

* Add Environ type to test_config.py

encode/uvicorn#1067 (comment)

* Add Literal type aliases for web server config

encode/uvicorn#1067 (comment)

* Use suggested casing for Literal type aliases

encode/uvicorn#1067 (comment)
encode/uvicorn#1067 (comment)

* Restore test_config.py test_app_factory comment

encode/uvicorn#1067 (comment)

Incorrectly modified in c436bba.

* Simplify event loop setup in config.py

encode/uvicorn#1067 (comment)

* Remove old type comment after merging master

encode/uvicorn#1067 (comment)

* Assert that certfile is present for SSL context

encode/uvicorn#1067 (comment)
https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain

`certfile` is a required argument for  `SSLContext.load_cert_chain`. As
it is currently, `is_ssl` could return `True` without `certfile`.

* Restore support for Config(loop='none')

encode/uvicorn#455
encode/uvicorn@e9e59e9
encode/uvicorn@e60ba66
encode/uvicorn#1067 (comment)

* Move WSGI types to uvicorn/_types.py

encode/uvicorn#1067 (comment)

* Remove Awaitable from app type annotation

encode/uvicorn#1067 (comment)

Co-authored-by: euri10 <euri10@users.noreply.github.com>
Kludex pushed a commit to Kludex/jik that referenced this pull request Aug 14, 2024
* Add changes from PR #992

This commit will bring in changes to uvicorn/config.py added by @Kludex
in PR #992, updating for the latest master branch.

* Correct SSL type annotations

encode/uvicorn#992
https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain

- `certfile` is a required positional argument when running
  `SSLContext.load_cert_chain`, so annotating as `Optional` (which
  allows `None`) would not be ideal. Path-like objects are acceptable,
  so after `from pathlib import Path`, the annotation is
  `certfile: Union[Path, str]`.
- `if self.is_ssl and self.ssl_certfile` will help ensure that the
  `self.ssl_certfile` required positional argument is present.

* Simplify log_config type annotation

`Dict[str, Any]` can be simplified to `dict`.

* Add type annotation for app

encode/uvicorn#990
encode/uvicorn#992

app is sometimes a string, as described in uvicorn/main.py. In other
cases, especially in the tests, app is an ASGI application callable.

This commit will add a type annotation for the app argument to address
these use cases.

* Simplify if expression in Config().headers

* Allow paths to be used for Config(env_file)

* Add asyncio Protocol types to class Config kwargs

Protocol classes are sometimes used for the app kwarg, such as in
tests/test_config.py.

* Improve use of Literal type for ASGI interfaces

https://mypy.readthedocs.io/en/stable/literal_types.html

This commit will retain the use of literals to define ASGI protocol
versions, but improve and correct the use of literal types.

As described in the mypy docs, `Literal["2.0", "3.0"]` is a simpler way
to write `Union[Literal["2.0"], Literal["3.0"]]`.

* Add type annotation to Config kwargs in workers.py

https://mypy.readthedocs.io/en/stable/type_inference_and_annotations.html

This is just one of those times when mypy needs a little help.

* Add type annotations to test_config.py

PR #978 added logic to convert `reload_dirs` from string to list, as
shown in `test_reload_dir_is_set()`, so the annotation on reload_dirs
will be updated to accept a string.

* Type-annotate constants in uvicorn/config.py

Prevents mypy `[has-type]` errors ("Cannot determine type of {object}")

* Correct type annotations in uvicorn/importer.py

encode/uvicorn#1046
encode/uvicorn#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.

* Use os.PathLike for paths in uvicorn/config.py

encode/uvicorn#1067 (comment)

Alternative to `pathlib.Path` introduced in Python 3.6.

* Use more specific types in test_config.py

encode/uvicorn#1067 (comment)
encode/uvicorn#1067 (comment)

https://github.com/encode/starlette/blob/b6f3578bb2cf6c60e3efe110143409b47f368d36/starlette/config.py#L16
https://github.com/python/cpython/blob/3e1c7167d86a2a928cdcb659094aa10bb5550c4c/Lib/os.py#L737
https://docs.pytest.org/en/latest/reference/reference.html#pytest.FixtureRequest

- Add custom types for exceptions and start response
- Use `MutableMapping` for `os.environ`: matches starlette/config.py.
- Use `pytest.FixtureRequest` for fixture requests. The `param`
  attribute is optional, so mypy requires a check for the attribute
  before indexing into it (`getattr(request, "param")`).

* Install missing YAML type stubs for mypy

encode/uvicorn#1067

Fixes `[import]` error: Library stubs not installed for "yaml"

* Add Environ type to test_config.py

encode/uvicorn#1067 (comment)

* Add Literal type aliases for web server config

encode/uvicorn#1067 (comment)

* Use suggested casing for Literal type aliases

encode/uvicorn#1067 (comment)
encode/uvicorn#1067 (comment)

* Restore test_config.py test_app_factory comment

encode/uvicorn#1067 (comment)

Incorrectly modified in c436bba.

* Simplify event loop setup in config.py

encode/uvicorn#1067 (comment)

* Remove old type comment after merging master

encode/uvicorn#1067 (comment)

* Assert that certfile is present for SSL context

encode/uvicorn#1067 (comment)
https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain

`certfile` is a required argument for  `SSLContext.load_cert_chain`. As
it is currently, `is_ssl` could return `True` without `certfile`.

* Restore support for Config(loop='none')

encode/uvicorn#455
encode/uvicorn@e9e59e9
encode/uvicorn@e60ba66
encode/uvicorn#1067 (comment)

* Move WSGI types to uvicorn/_types.py

encode/uvicorn#1067 (comment)

* Remove Awaitable from app type annotation

encode/uvicorn#1067 (comment)

Co-authored-by: euri10 <euri10@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"ModuleType object is not callable" warnings from 'importer'-imported modules
3 participants