fix: 3rd mypy issues #992
Closed
fix: 3rd mypy issues #992
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
|
Number of issues increased after trying to merge master back to it 😅 Turning it into a draft. |
br3ndonland
suggested changes
Apr 18, 2021
| timeout_notify: int = 30, | ||
| callback_notify: Callable[..., None] = None, | ||
| ssl_keyfile: Optional[str] = None, | ||
| ssl_certfile: Optional[str] = None, |
There was a problem hiding this comment.
As mentioned above, ssl_certfile: Optional[Union[Path, str]] = None, could work.
| assert not self.loaded | ||
|
|
||
| self.ssl: Optional[ssl.SSLContext] | ||
| if self.is_ssl: |
There was a problem hiding this comment.
Consider if self.is_ssl and self.ssl_certfile: instead, which will help ensure that the self.ssl_certfile required positional argument is present.
Merged
|
I'm closing this because it's a mess (too old, too many conflicts to fix) 😅 Thanks for the comments @br3ndonland. I'll take in consideration for my next PR. |
|
Sounds good. Maybe the Django ASGI types could be added in their own PR? It would be great to have some consensus on the ASGI types, so that we can build with them in other PRs. |
|
@br3ndonland What do you mean? |
|
That's right, I see that now. I guess most of the new types added to uvicorn/_types.py in this PR are no longer necessary, because we can just import them from |
euri10
added a commit
that referenced
this pull request
Jun 21, 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 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
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>
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I need help here 😗 I've found those two issues not trivial to solve. It's kind of a waterfall, I'm able to fix those two, but what comes after... 💥
I've tried to apply the ASGIApplication that we have on asgiref. But I ended up with mypy conflicts with the
test_lifespan.pyfile.Again... I can't target the right branch. On review consider only
7d6733cand after, please.