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

Force reload_dirs to be a list #978

Merged
merged 3 commits into from
Jun 1, 2021
Merged

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Mar 6, 2021

Fixes #976
If the user runs programmatically uvicorn he can do a: uvicorn.run(.....,reload_dirs="/home/maxou/dev/proj")
OP suggested we complain about that not being a list, I'd prefer we do it directly for him.
I added a test too
That kind of stuff makes me thing we should mypy the project, but this is another story

@Kludex
Copy link
Sponsor Member

Kludex commented Mar 7, 2021

I can fix the mypy issues we currently have so we can add it. Maybe duplicate some of the code that is on asgiref (django/asgiref#221) until its release? What do you think? @euri10

For reference, running mypy without any options:

$ mypy uvicorn/
...
Found 186 errors in 27 files (checked 38 source files)

@Kludex Kludex mentioned this pull request Mar 7, 2021
@euri10
Copy link
Member Author

euri10 commented Mar 8, 2021

It's a good idea @Kludex
I already started it a while ago and we decided to do it gradually to avoid reviewer headaches.
The idea is to add files that will be checked one by one in mypy.ini and fix them.

one by one or more if that makes more sense

@Kludex
Copy link
Sponsor Member

Kludex commented Mar 8, 2021

Ahhh! Ok, perfect. Makes sense. Thank you :)

@euri10 euri10 merged commit 22f78a7 into encode:master Jun 1, 2021
@euri10 euri10 deleted the reload_dir_str branch June 1, 2021 05:36
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 failing test

* Force reload_dirs to be a list

* Lint
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 failing test

* Force reload_dirs to be a list

* Lint
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>
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.

improve user experience by complaining when reload_dirs is given a string instead of a list
2 participants