Do async_setup_platform in background #36244

balloob · 2020-05-29T05:13:13Z

Breaking change

Proposed change

Make EntityComponent.async_setup(…) no longer wait until platform.async_setup_platform(…) is done.

This makes sense for various reasons:

Setting up a platform involves setting up the integration and then the entities. There is no reason for the entity component (ie light) to wait for this
We were waiting for async_setup_platform to finish, but async_add_entities was not part of what was awaited for.
We use hass.async_create_task so we still make sure they are finished before we finish startup.

I expect an even faster startup time now :)

It could happen previously that an integration was forwarding their config entry or making a discovery for sensor integration and then had to wait for sensor to set up all its platforms first (for no good reason).

CC @bdraco

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Example entry for `configuration.yaml`:

# Example configuration.yaml

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

MartinHjelmare · 2020-05-29T07:00:38Z

homeassistant/components/camera/__init__.py

@@ -273,7 +273,7 @@ async def preload_stream(hass, _):

            request_stream(hass, source, keepalive=True, options=camera.stream_options)

-    async_when_setup(hass, DOMAIN_STREAM, preload_stream)
+    hass.bus.async_listen_once(EVENT_HOMEASSISTANT_START, preload_stream)


Why isn't the old approach good? I'm just curious.

It's a bug. It needs to iterate over the entities. When stream is set up it's not ready with adding entities. So listening to START at least guarantees all platforms have set up their entities. It's still not perfect as camera entities might be loaded after start that also have preload stream activated…

MartinHjelmare · 2020-05-29T07:15:06Z

Regarding bullet number 2 in the description: In the entity platform helper setup we do wait for the entity addition that is done as part of the platform setup. If an entity is added later, that is not waited for.

pvizeli

Yeah make sense and make it real async

bdraco · 2020-05-29T14:43:30Z

Startup was slightly faster on my test install. Its mostly I/O bound / python load time (primarily the ecdsa module since we don't have gmpy2 installed and are doing the math in pure python) at this point.
2020-05-29 16:39:06 INFO (MainThread) [homeassistant.bootstrap] Home Assistant initialized in 10.77s

bdraco · 2020-05-29T14:46:36Z

The slow setup wait message now only shows what I would expect instead of climate and sensor being included as they are no longer blocked. 👍

2020-05-29 16:45:47 INFO (MainThread) [homeassistant.bootstrap] Waiting on integrations to complete setup: nws, nut

balloob · 2020-05-29T18:48:03Z

Regarding bullet number 2 in the description: In the entity platform helper setup we do wait for the entity addition that is done as part of the platform setup. If an entity is added later, that is not waited for.

You're right Martin. I have removed that code now :)

bdraco · 2020-05-29T19:41:13Z

Still working as expected for me

2020-05-29 21:21:45 INFO (MainThread) [homeassistant.bootstrap] Waiting on integrations to complete setup: nws, nut
2020-05-29 21:22:45 INFO (MainThread) [homeassistant.bootstrap] Waiting on integrations to complete setup: nws, nut
2020-05-29 21:23:45 INFO (MainThread) [homeassistant.bootstrap] Waiting on integrations to complete setup: nws, nut
2020-05-29 21:24:45 INFO (MainThread) [homeassistant.bootstrap] Waiting on integrations to complete setup: nws, nut

Tests are almost in good shape

MartinHjelmare · 2020-05-29T20:21:21Z

Are there any integrations that rely on waiting for the platform setup to add its entities before starting another task? Eg by awaiting forwarding the config entry.

This is a breaking change for integrations and most integrations don't have full test coverage.

balloob · 2020-05-29T20:29:47Z

That would not have been possible. Discory/forward is always done with async_create_task because of the circular dependency where hue needs to be set up for light.hue to be set up.

There are integrations that keep track when all platforms are loaded, but that will work the same as we are still waiting for all these tasks to finish.

MartinHjelmare · 2020-05-29T21:04:13Z

It's possible as you mention in the second paragraph. That's what I mean.

I don't think it's the same as now we can only wait for home assistant start. So integrations that assume that entities will be loaded when the platform setup is done may start the next task too early.

balloob · 2020-05-29T21:10:02Z

The only thing that is now being run in the background compared to before is users explicit platform configuration:

light:
  platform: bla

Discovery mechanism and forward config entry has not been changed, as it was already in the background.

MartinHjelmare · 2020-05-29T23:11:38Z

The important change is that the platform setup doesn't wait for entities now. Integrations that rely on that will experience breakage.

balloob · 2020-05-29T23:40:11Z

Integrations are unable to load their own platforms from async_setup because of a circular dependency (platforms require component to be set up).

So this would only break if an integration relies on another integration that is exposing entities. We don't have any. All our integrations providing automations do so in a generic manner and don't depend on setting up a specific integration.

I don't foresee this to be a problem.

MartinHjelmare · 2020-05-29T23:43:01Z

To be clear: In the existing code there's a straight await dependency from the entity component setup to the platform setup. There's no background scheduling in that line. We do need to schedule to the background the entry forwarding that starts the entity component setup but there's nothing stopping us from having another task wait for the forwarding to be done. If we assume that entities will be added when the entry forwarding is done, and we rely on that to start our second task, this PR will break that assumption.

balloob · 2020-05-29T23:43:21Z

Hmm maybe device_sun_light_trigger because it doesn't active its automation until Home Assistant is started (that event didn't exist when it was written)

balloob · 2020-05-29T23:48:35Z

Entry forwarding is not impacting by this PR. Forwarding an entry ends up calling EntityComponent.async_setup_entry. That whole chain is still intact without backgrounding.

This only impacts loading user supplied YAML platform config.

homeassistant/helpers/entity_platform.py

bdraco · 2020-05-30T02:17:56Z

diff --git a/tests/components/websocket_api/test_auth.py b/tests/components/websocket_api/test_auth.py
index a3b2fa1a6f..dc26ac06db 100644
--- a/tests/components/websocket_api/test_auth.py
+++ b/tests/components/websocket_api/test_auth.py
@@ -31,6 +31,7 @@ async def test_auth_events(
     )
 
     await test_auth_active_with_token(hass, no_auth_websocket_client, hass_access_token)
+    await hass.async_block_till_done()
 
     assert len(connected_evt) == 1
     assert not disconnected_evt

Fixed the websocket tests for me. Assumption is that the async_dispatcher_send job doesn't finish before the test checks

while pytest tests/components/websocket_api/test_*.py; do :; done

bdraco · 2020-05-30T14:48:37Z

Looks like one more here (but didn't fail locally for me)

--- a/tests/components/websocket_api/conftest.py
+++ b/tests/components/websocket_api/conftest.py
@@ -16,6 +16,7 @@ async def websocket_client(hass, hass_ws_client):
 async def no_auth_websocket_client(hass, aiohttp_client):
     """Websocket connection that requires authentication."""
     assert await async_setup_component(hass, "websocket_api", {})
+    await hass.async_block_till_done()
 
     client = await aiohttp_client(hass.http.app)
     ws = await client.ws_connect(URL)

bdraco · 2020-05-30T22:07:28Z

diff --git a/tests/components/local_file/test_camera.py b/tests/components/local_file/test_camera.py
index ce31ca5061..348bd94d32 100644
--- a/tests/components/local_file/test_camera.py
+++ b/tests/components/local_file/test_camera.py
@@ -25,6 +25,7 @@ async def test_loading_file(hass, hass_client):
                 }
             },
         )
+        await hass.async_block_till_done()
 
     client = await hass_client()
 
@@ -57,6 +58,7 @@ async def test_file_not_readable(hass, caplog):
                 }
             },
         )
+        await hass.async_block_till_done()
 
     assert "Could not read" in caplog.text
     assert "config_test" in caplog.text

The websocket api ones are stumping me.

bdraco · 2020-05-31T18:09:27Z

🤞 I think the last push on #36304 will fix it. Any time we call assert await async_setup_component(hass, "websocket_api", {}) and then block until done a second time in the same test (the no_auth_websocket_client does a setup as well) it was timing out because it was blocking forever

bdraco · 2020-05-31T19:20:38Z

Got a clean run on #36304. Taking out all the debug prints and pushing again

tests/components/openuv/test_config_flow.py

bdraco

161 files. That was a bit of a death march. Nice work 👍

bdraco · 2020-05-31T21:07:29Z

3.7 tests passed, for some reason it remade the 3.8 env so its taking longer

balloob · 2020-06-01T05:18:38Z

💃 💃 💃 💃 💃 💃 💃

Do async_setup_platform in background

de1f13e

homeassistant added the cla-signed label May 29, 2020

project-bot bot added this to Needs review in Dev May 29, 2020

probot-home-assistant bot added core new-feature small-pr PRs with less than 30 lines. labels May 29, 2020

Fix tests for entity integrations

694fc2d

MartinHjelmare reviewed May 29, 2020

View reviewed changes

pvizeli reviewed May 29, 2020

View reviewed changes

balloob added 2 commits May 29, 2020 11:28

Fix tests

0e24014

Fix more tests

b55172f

Remove entity platform waiting for entity tasks

98d243a

fix more tests

85d4632

balloob added 2 commits May 29, 2020 14:18

Fix tests episode 1000

f02b1a1

Fixes

ab5bf1e

Fix testssssss?

34af493

MartinHjelmare reviewed May 29, 2020

View reviewed changes

homeassistant/helpers/entity_platform.py Outdated Show resolved Hide resolved

balloob added 3 commits May 29, 2020 16:58

Restore the code to await first entities

c623434

Return True

9fc71ec

Test device sun light trigger activate on start

7060899

Update test_auth.py

04b0a75

Add block_till_done in websocket test

df1543b

bdraco mentioned this pull request May 30, 2020

Ignore this PR: Testing websocket tests in the CI that do not fail locally #36304

Closed

20 tasks

balloob added 2 commits May 30, 2020 21:55

Add local file await

7d5bf83

Use callbacks for dispatchers in test

362fbc1

Squash

4f47c49

bdraco reviewed May 31, 2020

View reviewed changes

tests/components/openuv/test_config_flow.py Show resolved Hide resolved

Dev automation moved this from Needs review to Reviewer approved May 31, 2020

bdraco approved these changes May 31, 2020

View reviewed changes

balloob merged commit 276f3af into dev Jun 1, 2020

Dev automation moved this from Reviewer approved to Done Jun 1, 2020

balloob deleted the async-setup-platform branch June 1, 2020 05:18

bdraco mentioned this pull request Jun 1, 2020

HTTP fails to start if cameras cannot be setup #35924

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do async_setup_platform in background #36244

Do async_setup_platform in background #36244

balloob commented May 29, 2020 •

edited

MartinHjelmare May 29, 2020

balloob May 29, 2020

MartinHjelmare commented May 29, 2020 •

edited

pvizeli left a comment

bdraco commented May 29, 2020 •

edited

bdraco commented May 29, 2020

balloob commented May 29, 2020

bdraco commented May 29, 2020

MartinHjelmare commented May 29, 2020

balloob commented May 29, 2020

MartinHjelmare commented May 29, 2020

balloob commented May 29, 2020

MartinHjelmare commented May 29, 2020

balloob commented May 29, 2020

MartinHjelmare commented May 29, 2020

balloob commented May 29, 2020

balloob commented May 29, 2020

bdraco commented May 30, 2020

bdraco commented May 30, 2020

bdraco commented May 30, 2020

bdraco commented May 31, 2020 •

edited

bdraco commented May 31, 2020 •

edited

bdraco left a comment •

edited

bdraco commented May 31, 2020

balloob commented Jun 1, 2020

Do async_setup_platform in background #36244

Do async_setup_platform in background #36244

Conversation

balloob commented May 29, 2020 • edited

Breaking change

Proposed change

Type of change

Example entry for configuration.yaml:

Additional information

Checklist

MartinHjelmare May 29, 2020

Choose a reason for hiding this comment

balloob May 29, 2020

Choose a reason for hiding this comment

MartinHjelmare commented May 29, 2020 • edited

pvizeli left a comment

Choose a reason for hiding this comment

bdraco commented May 29, 2020 • edited

bdraco commented May 29, 2020

balloob commented May 29, 2020

bdraco commented May 29, 2020

MartinHjelmare commented May 29, 2020

balloob commented May 29, 2020

MartinHjelmare commented May 29, 2020

balloob commented May 29, 2020

MartinHjelmare commented May 29, 2020

balloob commented May 29, 2020

MartinHjelmare commented May 29, 2020

balloob commented May 29, 2020

balloob commented May 29, 2020

bdraco commented May 30, 2020

bdraco commented May 30, 2020

bdraco commented May 30, 2020

bdraco commented May 31, 2020 • edited

bdraco commented May 31, 2020 • edited

bdraco left a comment • edited

Choose a reason for hiding this comment

bdraco commented May 31, 2020

balloob commented Jun 1, 2020

balloob commented May 29, 2020 •

edited

Example entry for `configuration.yaml`:

MartinHjelmare commented May 29, 2020 •

edited

bdraco commented May 29, 2020 •

edited

bdraco commented May 31, 2020 •

edited

bdraco commented May 31, 2020 •

edited

bdraco left a comment •

edited