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

run_app has incorrect typing #4957

Closed
iawells opened this issue Sep 10, 2020 · 4 comments
Closed

run_app has incorrect typing #4957

iawells opened this issue Sep 10, 2020 · 4 comments

Comments

@iawells
Copy link

iawells commented Sep 10, 2020

🐞 Describe the bug

async def _run_app(app: Union[Application, Awaitable[Application]], *,
                   host: Optional[str]=None,

The documentation is a bit woolly:

"host (str) – TCP/IP host or a sequence of hosts for HTTP server. Default is '0.0.0.0' if port has been specified or if path is not supplied."

('(str)' excludes 'None' and excludes 'a sequence of hosts'. Also, I don't think you mean 'sequence' literally.)

But it's clear from reading the code that an iterable of strings is planned for (along with several other types).

I would think you want Optional[Union[str, Iterable[str]]] - though given the following code it's maybe unclear if you think other types are also acceptable.

πŸ’‘ To Reproduce

Write some code like this and then run it though mypy

from aiohttp.web import run_app

hosts: List[str] = ['a', 'b']

run_app(host=hosts)

πŸ’‘ Expected behavior

If a list of strings is acceptable, then the attribute is wrong. I'm surprised mypy didn't spot it during CI, actually.

πŸ“‹ Your version of the Python

$ python --version
Python 3.6.9

πŸ“‹ Your version of the aiohttp/yarl/multidict distributions

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.6.2
$ python -m pip show multidict
Name: multidict
Version: 4.7.6
$ python -m pip show yarl
Name: yarl
Version: 1.5.1
@iawells iawells added the bug label Sep 10, 2020
@WisdomPill
Copy link
Member

Hello @iawells !

Nice catch!
Can you open a PR to address the issue?

@WisdomPill
Copy link
Member

Otherwise I can step in and do it

derlih added a commit to derlih/aiohttp that referenced this issue Oct 24, 2020
@derlih derlih mentioned this issue Oct 24, 2020
5 tasks
asvetlov added a commit that referenced this issue Oct 24, 2020
asvetlov added a commit that referenced this issue Oct 24, 2020
@derlih
Copy link
Contributor

derlih commented Oct 24, 2020

@asvetlov shouldn't this issue be closed?

@asvetlov
Copy link
Member

Yes, sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants