-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
Switch to using the AsyncResolver with aiohttp #114529
Conversation
This avoids creating executor jobs to do DNS resolution AsyncResolver was not usable before aio-libs/aiohttp#8270 because it did not fallback to fallback to A records and only returned AAAA records in most cases when IPv6 was available This is a backport of aio-libs/aiohttp#8270
This avoids creating executor jobs to do DNS resolution AsyncResolver was not usable before aio-libs/aiohttp#8270 because it did not fallback to fallback to A records and only returned AAAA records in most cases when IPv6 was available This is a backport of aio-libs/aiohttp#8270
This avoids creating executor jobs to do DNS resolution AsyncResolver was not usable before aio-libs/aiohttp#8270 because it did not fallback to fallback to A records and only returned AAAA records in most cases when IPv6 was available This is a backport of aio-libs/aiohttp#8270
This avoids creating executor jobs to do DNS resolution AsyncResolver was not usable before aio-libs/aiohttp#8270 because it did not fallback to fallback to A records and only returned AAAA records in most cases when IPv6 was available This is a backport of aio-libs/aiohttp#8270
Hey there @gjohansson-ST, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
Test failures are likely unmocked http calls in integrations. Since it ran in the executor before the executor would be shut down and it wouldn't linger, but since dns resolution is shielded, they're waiting to finish |
extracted from #114539
Since the upstream PR isn't merged yet, I suggest we draft this PR. If we want to add it before the upstream PR is merged, it's not a backport so at minimum things need to be renamed. |
Will get the upstream PR merged soon. Got distracted with other issues and didn't come back to it. It should be ready to merge, but I'm double checking that nobody else has review feedback before I do that. |
Got busy with release issues so I didn't get to finish double checking the aiohttp PR until today. Its merged now so this one should be good to go now |
Needs some tests fixed
Proposed change
AsyncResolver
avoids creating executor jobs to do DNS resolution. This is key to further reducing startup time, as the DNS cache is not built yet, and integrations are heavily competing for the executor. Additionally, having to create executor jobs to make async HTTP connections negatively impacts performance and latency.https://docs.aiohttp.org/en/stable/#library-installation
AsyncResolver
was not usable before aio-libs/aiohttp#8270 because it did not fallback to A records and only returned AAAA records in most cases when IPv6 was available.This is a backport of aio-libs/aiohttp#8270
Making aiodns a top level dep should have very minimal impact as aiodns is already required by dhcp, which is required by default config
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.To help with the load of incoming pull requests: