Retrying connecting Influxdb at setup #22567

SndR85 · 2019-03-30T18:06:35Z

Description:

This change makes the Influxdb setup more robust. I had the issue that Influxdb itself started slower than Home-Assistant, even with max_retries set That resulted in a not functioning Influxdb component. I thought max_retries should fix this, but I saw that setting max_retries only affects at the moment of writing to Influxdb, not in setup(). With this change, the max_retries is also used at setup time.

There is no config change with current change.

homeassistant · 2019-03-30T18:06:37Z

Hello @scornelissen85,

When attempting to inspect the commits of your pull request for CLA signature status among all authors we encountered commit(s) which were not linked to a GitHub account, thus not allowing us to determine their status(es).

The commits that are missing a linked GitHub account are the following:

df536341593449494fdfd5068f03b9cc2023e257 - This commit has something that looks like an email address (scornelissen@quintor.local). Maybe try linking that to GitHub?.

Unfortunately, we are unable to accept this pull request until this situation is corrected.

Here are your options:

If you had an email address set for the commit that simply wasn't linked to your GitHub account you can link that email now and it will retroactively apply to your commits. The simplest way to do this is to click the link to one of the above commits and look for a blue question mark in a blue circle in the top left. Hovering over that bubble will show you what email address you used. Clicking on that button will take you to your email address settings on GitHub. Just add the email address on that page and you're all set. GitHub has more information about this option in their help center.
If you didn't use an email address at all, it was an invalid email, or it's one you can't link to your GitHub, you will need to change the authorship information of the commit and your global Git settings so this doesn't happen again going forward. GitHub provides some great instructions on how to change your authorship information in their help center.
- If you only made a single commit you should be able to run
```
git commit --amend --author="Author Name <email@address.com>"
```
  (substituting Author Name and email@address.com for your actual information) to set the authorship information.
- If you made more than one commit and the commit with the missing authorship information is not the most recent one you have two options:
  1. You can re-create all commits missing authorship information. This is going to be the easiest solution for developers that aren't extremely confident in their Git and command line skills.
  2. You can use this script that GitHub provides to rewrite history. Please note: this should be used only if you are very confident in your abilities and understand its impacts.
- Whichever method you choose, I will come by to re-check the pull request once you push the fixes to this branch.

We apologize for this inconvenience, especially since it usually bites new contributors to Home Assistant. We hope you understand the need for us to protect ourselves and the great community we all have built legally. The best thing to come out of this is that you only need to fix this once and it benefits the entire Home Assistant and GitHub community.

Thanks, I look forward to checking this PR again soon! ❤️

homeassistant · 2019-03-30T19:14:29Z

Hi @scornelissen85,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

dgomes · 2019-03-30T22:52:11Z

~~We already have a way for reconnecting to servers when they are not ready.~~

~~you should just raise PlatformNotReady~~

~~check existing home-assistant components that implement this (e.g. dlna_dmr)~~

@amelchio is right, sorry ☹️

amelchio · 2019-03-30T23:40:31Z

Since this is not an entity platform, PlatformNotReady will not work.

amelchio

Hello and welcome 😀. This approach is unfortunately not acceptable.

In the except handler, you can use homeassistant.helpers.event.call_later() to schedule another setup() something like one minute later and then just return True

Let me know if you need help with that :)

amelchio · 2019-03-30T23:44:51Z

homeassistant/components/influxdb/__init__.py

+        except (exceptions.InfluxDBClientError, IOError,
+                requests.exceptions.ConnectionError) as exc:
+            if retry < max_tries:
+                time.sleep(RETRY_DELAY)


We are not yet on the InfluxThread, so we are not allowed to sleep here (since it blocks other tasks from using this thread).

SndR85 · 2019-03-31T08:01:30Z

Thanks for your input. I will change it to using homeassistant.helpers.event.call_later(), as that will not block the main thread I guess. I noticed that the sleep in setup slows down the startup, as amelchio noticed.

Should I made a new PR?

amelchio · 2019-03-31T08:43:43Z

That is up to you, feel free to keep pushing to this one.

houndci-bot · 2019-03-31T09:14:35Z

homeassistant/components/influxdb/__init__.py

+        _LOGGER.warning("Database host is not accessible due to '%s', please "
+                        "check your entries in the configuration file (host, "
+                        "port, etc.) and verify that the database exists and is "
+                        "READ/WRITE. Retrying again in %s sec.", exc, RETRY_INTERVAL)


line too long (85 > 79 characters)

houndci-bot · 2019-03-31T09:14:35Z

homeassistant/components/influxdb/__init__.py

-        return False
+        _LOGGER.warning("Database host is not accessible due to '%s', please "
+                        "check your entries in the configuration file (host, "
+                        "port, etc.) and verify that the database exists and is "


line too long (81 > 79 characters)

houndci-bot · 2019-03-31T09:14:36Z

homeassistant/components/influxdb/__init__.py

@@ -39,6 +39,7 @@
 TIMEOUT = 5
 RETRY_DELAY = 20
 QUEUE_BACKLOG_SECONDS = 30
+RETRY_INTERVAL = 60 # seconds


at least two spaces before inline comment

amelchio

Yes, this is better 👍. Just a couple of small comments.

amelchio · 2019-03-31T10:22:04Z

homeassistant/components/influxdb/__init__.py

+        )
+        event_helper.call_later(
+            hass, RETRY_INTERVAL, lambda _: setup(hass, config)
+        )


You should return True here or the code below will create a new thread for each retry.

Fixed it by my latest commit

amelchio · 2019-03-31T10:23:22Z

homeassistant/components/influxdb/__init__.py

+            "Database host is not accessible due to '%s', please "
+            "check your entries in the configuration file (host, "
+            "port, etc.) and verify that the database exists and is "
+            "READ/WRITE. Retrying again in %s sec.", exc, RETRY_INTERVAL


Please type out "seconds"

Fixed it by my latest commit

amelchio

Nice 👍

SndR85 · 2019-04-08T19:26:10Z

Just a small question, maybe more general about the release process. When will this PR being released in stable Home Assistant?

amelchio · 2019-04-08T19:47:41Z

April 17th.

There is a release every two weeks and before that you must spend a week in beta. You missed the previous beta by a few days.

This PR will be in the beta that ships about two days from now and it will be in stable a week after that (releases are in the middle of the week, usually Wednesday).

SndR85 · 2019-04-13T15:12:25Z

Thanks

SndR85 requested a review from fabaff as a code owner March 30, 2019 18:06

homeassistant added cla-error integration: influxdb small-pr PRs with less than 30 lines. labels Mar 30, 2019

ghost added the in progress label Mar 30, 2019

Also retry Influxdb at setup()

4a8da64

SndR85 force-pushed the influxdb-setup-tries branch from df53634 to 4a8da64 Compare March 30, 2019 19:14

homeassistant added cla-needed cla-signed and removed cla-needed labels Mar 30, 2019

amelchio added cla-recheck and removed cla-error cla-signed labels Mar 30, 2019

homeassistant added cla-signed and removed cla-recheck labels Mar 30, 2019

amelchio requested changes Mar 30, 2019

View reviewed changes

Use event.call_later() for retry setup Influxdb

a113eb9

houndci-bot reviewed Mar 31, 2019

View reviewed changes

SndR85 added 2 commits March 31, 2019 11:15

Fix max line length in setup() in Influxdb

227d1c9

Add extra space before comment

f017560

amelchio requested changes Mar 31, 2019

View reviewed changes

Fix sec -> seconds and add return True

5f2ec30

amelchio approved these changes Mar 31, 2019

View reviewed changes

cgarwood merged commit 7d7b931 into home-assistant:dev Mar 31, 2019

ghost removed the in progress label Mar 31, 2019

stickystyle mentioned this pull request Apr 1, 2019

Influxdb does not work properly after reboot #18094

Closed

home-assistant locked as resolved and limited conversation to collaborators Apr 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retrying connecting Influxdb at setup #22567

Retrying connecting Influxdb at setup #22567

SndR85 commented Mar 30, 2019

homeassistant commented Mar 30, 2019

homeassistant commented Mar 30, 2019

dgomes commented Mar 30, 2019 •

edited

Loading

amelchio commented Mar 30, 2019

amelchio left a comment

amelchio Mar 30, 2019

SndR85 commented Mar 31, 2019 •

edited

Loading

amelchio commented Mar 31, 2019

houndci-bot Mar 31, 2019

houndci-bot Mar 31, 2019

houndci-bot Mar 31, 2019

amelchio left a comment

amelchio Mar 31, 2019

SndR85 Mar 31, 2019

amelchio Mar 31, 2019

SndR85 Mar 31, 2019

amelchio left a comment

SndR85 commented Apr 8, 2019

amelchio commented Apr 8, 2019

SndR85 commented Apr 13, 2019

Retrying connecting Influxdb at setup #22567

Retrying connecting Influxdb at setup #22567

Conversation

SndR85 commented Mar 30, 2019

Description:

homeassistant commented Mar 30, 2019

homeassistant commented Mar 30, 2019

dgomes commented Mar 30, 2019 • edited Loading

amelchio commented Mar 30, 2019

amelchio left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SndR85 commented Mar 31, 2019 • edited Loading

amelchio commented Mar 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amelchio left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amelchio left a comment

Choose a reason for hiding this comment

SndR85 commented Apr 8, 2019

amelchio commented Apr 8, 2019

SndR85 commented Apr 13, 2019

dgomes commented Mar 30, 2019 •

edited

Loading

SndR85 commented Mar 31, 2019 •

edited

Loading