Cancel config entry retry, platform retry, and polling at the stop event by bdraco · Pull Request #49138 · home-assistant/core

bdraco · 2021-04-12T23:54:48Z

Proposed change

The retries and polling would delay shutdown because the retry would start during phase 1 shutdown and create new tracked tasks.

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

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

bdraco · 2021-04-13T00:00:09Z

Edit: nm, bad testing on my part.

bdraco · 2021-04-13T00:07:08Z

Things we still wait for

2021-04-13 00:06:39 DEBUG (MainThread) [homeassistant.core] Waited 1 seconds for task: <Task pending name='Task-12451' coro=<async_setup.<locals>.stop_server() running at /usr/src/homeassistant/homeassistant/components/http/__init__.py:162> wait_for=<_GatheringFuture pending cb=[<TaskWakeupMethWrapper object at 0x7fdad7ec24f0>()]>>
2021-04-13 00:06:40 DEBUG (MainThread) [homeassistant.core] Waited 2 seconds for task: <Task pending name='Task-12451' coro=<async_setup.<locals>.stop_server() running at /usr/src/homeassistant/homeassistant/components/http/__init__.py:162> wait_for=<_GatheringFuture pending cb=[<TaskWakeupMethWrapper object at 0x7fdad7ec24f0>()]>>
2021-04-13 00:06:41 DEBUG (MainThread) [homeassistant.core] Waited 1 seconds for task: <Task pending name='Task-12847' coro=<Scanner.async_scan() running at /usr/src/homeassistant/homeassistant/components/ssdp/__init__.py:81> wait_for=<Future pending cb=[<TaskWakeupMethWrapper object at 0x7fdadd098ee0>()]>>
2021-04-13 00:06:42 DEBUG (MainThread) [homeassistant.core] Waited 2 seconds for task: <Task pending name='Task-12847' coro=<Scanner.async_scan() running at /usr/src/homeassistant/homeassistant/components/ssdp/__init__.py:81> wait_for=<Future pending cb=[<TaskWakeupMethWrapper object at 0x7fdadd098ee0>()]>>

fixed in #49140

bdraco · 2021-04-13T00:19:41Z

Looks like we keep updating entities as well after the stop event since the interval listener isn't canceled

bdraco · 2021-04-13T00:23:32Z

We still send up waiting for the executor if there is a sync connect in progress on shutdown, but at least we won't be adding more of them

Process 213: /usr/local/bin/python3 /usr/src/homeassistant/homeassistant/__main__.py --config /config
Python v3.8.7 (/usr/local/bin/python3.8)

Thread 213 (idle): "MainThread"
    _wait_for_tstate_lock (threading.py:1027)
    join (threading.py:1011)
    shutdown (concurrent/futures/thread.py:236)
    close (homeassistant/runner.py:92)
    run (asyncio/runners.py:51)
    run (homeassistant/runner.py:126)
    main (homeassistant/__main__.py:313)
    <module> (homeassistant/__main__.py:321)
Thread 437 (idle): "Thread-8"
    loop (paho/mqtt/client.py:1167)
    loop_forever (paho/mqtt/client.py:1779)
    _thread_main (paho/mqtt/client.py:3452)
    run (threading.py:870)
    _bootstrap_inner (threading.py:932)
    _bootstrap (threading.py:890)
Thread 438 (idle): "Thread-9"
    loop (paho/mqtt/client.py:1167)
    loop_forever (paho/mqtt/client.py:1779)
    _thread_main (paho/mqtt/client.py:3452)
    run (threading.py:870)
    _bootstrap_inner (threading.py:932)
    _bootstrap (threading.py:890)
Thread 442 (idle): "SyncWorker_15"
    _open_socket (websocket/_http.py:176)
    connect (websocket/_http.py:121)
    connect (websocket/_core.py:222)
    create_connection (websocket/_core.py:515)
    open (samsungtvws/remote.py:146)
    try_connect (homeassistant/components/samsungtv/bridge.py:229)
    _try_connect (homeassistant/components/samsungtv/config_flow.py:116)
    run (concurrent/futures/thread.py:57)
    _worker (concurrent/futures/thread.py:80)
    run (threading.py:870)
    _bootstrap_inner (threading.py:932)
    _bootstrap (threading.py:890)

bdraco · 2021-04-13T00:56:27Z

Hmm... I wonder if we can abort the flow but probably not since its happening in the executor

2021-04-13 00:54:36 DEBUG (MainThread) [homeassistant.core] Waited 90 seconds for task: <Future pending cb=[_chain_future.<locals>._call_check_cancel() at /usr/local/lib/python3.8/asyncio/futures.py:360, <TaskWakeupMethWrapper object at 0x7f67b857ab50>()]>
2021-04-13 00:54:37 DEBUG (MainThread) [homeassistant.core] Waited 91 seconds for task: <Future pending cb=[_chain_future.<locals>._call_check_cancel() at /usr/local/lib/python3.8/asyncio/futures.py:360, <TaskWakeupMethWrapper object at 0x7f67b857ab50>()]>
2021-04-13 00:54:38 DEBUG (MainThread) [homeassistant.core] Waited 92 seconds for task: <Future pending cb=[_chain_future.<locals>._call_check_cancel() at /usr/local/lib/python3.8/asyncio/futures.py:360, <TaskWakeupMethWrapper object at 0x7f67b857ab50>()]>
2021-04-13 00:54:39 DEBUG (MainThread) [homeassistant.core] Waiting for task: <Future finished exception=AbortFlow('Flow aborted: cannot_connect')>

The retries would block shutdown.

bdraco · 2021-04-13T22:36:34Z

I need to do functional testing on this before merge so marking this as a draft

bdraco · 2021-04-14T02:16:05Z

Retest with it moved looks good

MartinHjelmare · 2021-04-14T09:10:30Z

        else:
            self.state = ENTRY_STATE_SETUP_ERROR

+    async def async_shutdown(self) -> None:


Why should this be a coroutine function? We don't await inside.

I made it a coroutine because I figured we would need to add more to it later (#49241) and it would end up being a coroutine anyways which would be a breaking change in the api I wanted to avoid in the future.

Sounds good!

MartinHjelmare · 2021-04-14T09:11:21Z


        await self._async_setup_platform(async_create_setup_task)

+    async def async_shutdown(self) -> None:


homeassistant added the cla-signed label Apr 12, 2021

probot-home-assistant Bot added the core label Apr 12, 2021

bdraco changed the title ~~Cancel config entry and platform retry at the stop event~~ Cancel config entry and platform retry, and polling at the stop event Apr 13, 2021

bdraco mentioned this pull request Apr 13, 2021

Add samsungtv dhcp and zeroconf discovery #48022

Merged

21 tasks

bdraco changed the title ~~Cancel config entry and platform retry, and polling at the stop event~~ Cancel config entry retry, platform retry, and polling at the stop event Apr 13, 2021

bdraco mentioned this pull request Apr 13, 2021

Stop ssdp scans when stop event happens #49140

Merged

21 tasks

bdraco marked this pull request as ready for review April 13, 2021 08:19

bdraco mentioned this pull request Apr 13, 2021

Avoid sending empty integration list multiple times during subscribe_bootstrap_integrations #49181

Merged

21 tasks

balloob reviewed Apr 13, 2021

View reviewed changes

Comment thread homeassistant/helpers/entity_platform.py Outdated

balloob approved these changes Apr 13, 2021

View reviewed changes

bdraco added 7 commits April 13, 2021 12:14

Cancel config entry and platform retry at the stop event

a91568f

The retries would block shutdown.

.. and stop polling

1be73df

coverage

6cd17ef

coverage

7d89af9

coverage

d9059ba

coverage

2586d3c

move entity platform shutdown to be per component

9c15bde

bdraco force-pushed the abort_setup_retry_on_stop_event branch from 1526aa1 to 9c15bde Compare April 13, 2021 22:15

fix

77de50a

bdraco marked this pull request as draft April 13, 2021 22:36

bdraco added the Testing required label Apr 13, 2021

bdraco commented Apr 13, 2021

View reviewed changes

Comment thread homeassistant/helpers/entity_platform.py

bdraco commented Apr 14, 2021

View reviewed changes

Comment thread homeassistant/helpers/entity_component.py Outdated

Remove unreachable code

1329d3f

bdraco marked this pull request as ready for review April 14, 2021 02:16

bdraco merged commit 44beff3 into home-assistant:dev Apr 14, 2021

bdraco deleted the abort_setup_retry_on_stop_event branch April 14, 2021 02:16

MartinHjelmare reviewed Apr 14, 2021

View reviewed changes

github-actions Bot locked and limited conversation to collaborators Apr 16, 2021


		await self._async_setup_platform(async_create_setup_task)

		async def async_shutdown(self) -> None:

Uh oh!

Conversation

bdraco commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Type of change

Example entry for configuration.yaml:

Additional information

Checklist

Uh oh!

bdraco commented Apr 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdraco commented Apr 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdraco commented Apr 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdraco commented Apr 13, 2021

Uh oh!

bdraco commented Apr 13, 2021

Uh oh!

Uh oh!

bdraco commented Apr 13, 2021

Uh oh!

Uh oh!

Uh oh!

bdraco commented Apr 14, 2021

Uh oh!

MartinHjelmare Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

bdraco Apr 15, 2021

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare Apr 15, 2021

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bdraco commented Apr 12, 2021 •

edited

Loading

Example entry for `configuration.yaml`:

bdraco commented Apr 13, 2021 •

edited

Loading

bdraco commented Apr 13, 2021 •

edited

Loading

bdraco commented Apr 13, 2021 •

edited

Loading