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

EntityComponent to retry platforms that are not ready yet #8209

Merged
merged 3 commits into from
Jun 26, 2017

Conversation

balloob
Copy link
Member

@balloob balloob commented Jun 25, 2017

Description:

This adds support for the entity component to retry setup of platforms that are not ready yet.

  • Introduces a new exception PlatformNotReady that platforms can raise to opt-in for the built-in retry logic.
  • EntityComponent catches this exception and will automatically retry the platform setup up to 10 times. Time between retries is 5, 10, 15, 20, 25 and 30 seconds after each retry after that.
  • After discussion with @armills on Gitter: time between retries has been changed to 30 seconds * # of tries with a max time of 5 minutes. There is no cap on the max retries.

Related issue (if applicable): fixes #4937

Checklist:

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link

@balloob, thanks for your PR! By analyzing the history of the files in this pull request, we identified @pvizeli, @fabaff and @partofthething to be potential reviewers.

@emlove
Copy link
Contributor

emlove commented Jun 26, 2017

I think we should back off to some longer interval (5 to 30 minutes), then retry perpetually. Especially if this is going to be opt-in by platforms. The big use-case is platforms that aren't available 24/7, and require a restart to pick up. If hass is only retrying for a short window after start up, this doesn't really fix it. The big concern is that as soon as you have more than one platform that has to be online during start up, it becomes trickier to find a window where everything is online and you can successfully start up.

Copy link
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

I think we should try it out and maby with new expirence we see points to make it better. But it Looks greate 👍

Copy link
Contributor

@emlove emlove left a comment

Choose a reason for hiding this comment

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

I agree. Especially since this is opt-in it doesn't need to be perfect right away. We can tweak it later as necessary and from what we learn.

@balloob balloob merged commit d73b695 into dev Jun 26, 2017
@balloob balloob deleted the platform-not-ready branch June 26, 2017 16:41
@balloob balloob mentioned this pull request Jul 1, 2017
@lwis lwis mentioned this pull request Jul 8, 2017
3 tasks
dethpickle pushed a commit to dethpickle/home-assistant that referenced this pull request Aug 18, 2017
…tant#8209)

* Add PlatformNotReady Exception

* lint

* Remove cap, adjust algorithm
@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better error handling in platforms
6 participants