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

Intelligent timeout handler for setup/bootstrap #38329

Merged
merged 81 commits into from
Aug 5, 2020
Merged

Conversation

pvizeli
Copy link
Member

@pvizeli pvizeli commented Jul 28, 2020

Proposed change

This timeout handler solve forever all issues with blocking startup setups without a hack or workaround. It's a zone/global-based timeout handler.

ToDo:

  • Init in early-stage and share it over hass.data
  • Wrap every boot stage with a global timeout - 5min
  • On component/integration setup, use a zone (domain) based timeout of 2min (or 1min?)
  • Wrap pip install with a freeze
  • Wrap database migration with a freeze

This domain allows a clean setup and respect all kind of timeouts and raise only a timeout if a component make issues.
This needs some tweaks and optimization, all help is welcome.

async with hass.data["timeout"].async_timeout(300):
    await hass.async_block_till_done()

async with hass.data["timeout"].async_timeout(300, cool_down=60):
    await hass.async_block_till_done()  # give 1min time to finish after zones are done


# Or zone-based

async with hass.data["timeout"].async_timeout(120, "domain"):
    await integration.async_setup_component()
async with hass.data["timeout"].freeze():
    await pip.install(...)

with hass.data["timeout"].freeze():
    migrate_database()

# Or zone-based

with hass.data["timeout"].freeze("recorder"):
    migrate_database()

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:

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

@pvizeli pvizeli marked this pull request as draft July 28, 2020 16:08
@pvizeli pvizeli requested a review from balloob July 30, 2020 07:27
@pvizeli
Copy link
Member Author

pvizeli commented Jul 30, 2020

So, the timeout is now done with all supported features. Now we need just add it to HA

@pvizeli
Copy link
Member Author

pvizeli commented Jul 30, 2020

Add added a cool down which allow async_block_till_done() to finish things after the latest zone/child was finished

@pvizeli pvizeli mentioned this pull request Aug 3, 2020
20 tasks
@MartinHjelmare MartinHjelmare changed the title intelligence timeout handler for setup/bootstrap Intelligent timeout handler for setup/bootstrap Aug 5, 2020
@bdraco bdraco marked this pull request as ready for review August 5, 2020 05:50
@bdraco
Copy link
Member

bdraco commented Aug 5, 2020

@pvizeli I did as much testing as I could think of with this. So far everything looks good and I put it on both my houston and maui primary instances without any issues.

Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@bdraco
Copy link
Member

bdraco commented Aug 5, 2020

Retesting now

Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Installed on 5 production instances. No obvious issues

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

Successfully merging this pull request may close these issues.

0.113.2 - Home Assistant is starting, not everything will be available until it is finished.
4 participants