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

Add optional support for aiodns to TCPConnector for async DNS resolving #728

Closed
asvetlov opened this issue Jan 10, 2016 · 6 comments
Closed

Comments

@asvetlov
Copy link
Member

See http://stackoverflow.com/questions/34700311/how-to-handle-dns-timeouts-with-aiohttp for reasoning

@gwillem
Copy link
Contributor

gwillem commented Jan 10, 2016

Thanks for your suggestions! To be solved:

  1. The HTTP host header needs to be overwritten (easy)
  2. SSL verification requires the true hostname. I have dug in the code but not found a way yet to cleanly pass the hostname to the SSL context (relevant code here)

@asvetlov
Copy link
Member Author

TCPConnector._resolve https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/connector.py#L531 performs DNS lookup if use_dns_cache for TCPConnector http://aiohttp.readthedocs.org/en/stable/client_reference.html#tcpconnector is True.
I suggest adding new ctor parameter use_aiodns (also False by default) to resolve through aiodns if use_dns_cache is True.

@fafhrd91
Copy link
Member

-1 for use_aiodns, better is to invert dependency.
we need Resolver object, and pass that object as parameter to connector, then we can create default resolver that uses asyncio and aiodns one.

On Jan 10, 2016, at 4:26 AM, Andrew Svetlov notifications@github.com wrote:

TCPConnector._resolve https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/connector.py#L531 https://github.com/KeepSafe/aiohttp/blob/master/aiohttp/connector.py#L531 performs DNS lookup if use_dns_cache for TCPConnector http://aiohttp.readthedocs.org/en/stable/client_reference.html#tcpconnector http://aiohttp.readthedocs.org/en/stable/client_reference.html#tcpconnector is True.
I suggest adding new ctor parameter use_aiodns (also False by default) to resolve through aiodns if use_dns_cache is True.


Reply to this email directly or view it on GitHub #728 (comment).

@asvetlov
Copy link
Member Author

Yes, Resolver sounds better than use_aiodns.
Thanks for correction.

@gwillem gwillem mentioned this issue Jan 24, 2016
2 tasks
@lphuberdeau lphuberdeau mentioned this issue Mar 10, 2016
1 task
@asvetlov
Copy link
Member Author

asvetlov commented Jun 3, 2016

Fixed by #819

@asvetlov asvetlov closed this as completed Jun 3, 2016
@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants