Skip to content

Fix update on cert_expiry startup#27137

Merged
MartinHjelmare merged 6 commits into
home-assistant:devfrom
jjlawren:cert_update_once
Oct 3, 2019
Merged

Fix update on cert_expiry startup#27137
MartinHjelmare merged 6 commits into
home-assistant:devfrom
jjlawren:cert_update_once

Conversation

@jjlawren
Copy link
Copy Markdown
Contributor

@jjlawren jjlawren commented Oct 3, 2019

Description:

Cert Expiry sensors are now updated when the entities are created if home assistant is running or at home assistant start if home assistant is not running yet.

Import of config yaml is delayed and done at home assistant start, to avoid issues with checking certificate served by home assistant.

Related issue (if applicable): may fix #26740

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

Comment thread homeassistant/components/cert_expiry/sensor.py
@jjlawren
Copy link
Copy Markdown
Contributor Author

jjlawren commented Oct 3, 2019

I'm guessing the receiving server didn't like to accept multiple connections from the same source in quick succession, rejecting the new connections and causing the entity to fall into a bad state. It sounds like they updated fine at the next 12 hour update.

@MartinHjelmare
Copy link
Copy Markdown
Member

Yeah I think we shouldn't update twice close like we do now. But which update should we remove?

@jjlawren
Copy link
Copy Markdown
Contributor Author

jjlawren commented Oct 3, 2019

The self-connecting scenario is one I hadn't considered. Probably should swap back to updating when HA is fully started. I'll do that shortly.

@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented Oct 3, 2019

Yeah. Let's create the entity as available and remove the update during entity addition. Keep the update on home assistant start.

The entity will then be created with unknown state. When the first update happens, either on home assistant start or 12 hours after creating the entity, the entity will get correct state.

I guess we could add some more complex logic in async_added_to_hass that checks the run state of home assistant. If it's already running, we schedule an update immediately. If home assistant is setting up, we schedule an update on home assistant start event.

@MartinHjelmare
Copy link
Copy Markdown
Member

To solve the problem with importing the config yaml, we could schedule the import on home assistant start event.

Comment thread homeassistant/components/cert_expiry/sensor.py
Comment thread homeassistant/components/cert_expiry/sensor.py Outdated
Comment thread homeassistant/components/cert_expiry/sensor.py Outdated
Comment thread homeassistant/components/cert_expiry/sensor.py Outdated
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Nice!

@MartinHjelmare
Copy link
Copy Markdown
Member

Do you want to change config yaml import scheduling in a separate PR?

@jjlawren
Copy link
Copy Markdown
Contributor Author

jjlawren commented Oct 3, 2019

I removed that since it didn't seem necessary anymore with the conditional updates when the entities are added. Do you think something is still needed there?

@jjlawren
Copy link
Copy Markdown
Contributor Author

jjlawren commented Oct 3, 2019

You're right, I forgot the import would attempt to validate the configuration. If it was checking itself this would cause the import to fail. Updated the import step to delay until HA is fully started.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Great!

@MartinHjelmare MartinHjelmare changed the title Don't force redundant update on cert_expiry startup Fix update on cert_expiry startup Oct 3, 2019
@MartinHjelmare MartinHjelmare merged commit bb45bdd into home-assistant:dev Oct 3, 2019
@jjlawren jjlawren deleted the cert_update_once branch October 3, 2019 15:41
@lock lock Bot locked and limited conversation to collaborators Oct 4, 2019
@balloob balloob added this to the 0.100 milestone Oct 7, 2019
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.

Certificate Expiry component

5 participants