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

Handle connection drops when upgrading #6767

Merged
merged 6 commits into from
Sep 2, 2020
Merged

Handle connection drops when upgrading #6767

merged 6 commits into from
Sep 2, 2020

Conversation

ludeeus
Copy link
Member

@ludeeus ludeeus commented Sep 2, 2020

Breaking change

Proposed change

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

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.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@@ -33,6 +34,11 @@ class HassioDashboard extends LitElement {

@property({ attribute: false }) public hassOsInfo!: HassioHassOSInfo;

protected firstUpdated(changedProps) {
super.firstUpdated(changedProps);
this._postUpdateDialog();
Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a good event that can be listened on here?
Currently, if the user now refreshes while doing the upgrade this will trigger, I guess that is fine.

I tried listening to the connection -> ready event, but that fires too early, the dialogs would not show.

Copy link
Member

Choose a reason for hiding this comment

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

It will also trigger when I navigated away during the download/update and come back a week later, that is weird I think?

Copy link
Member

Choose a reason for hiding this comment

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

If we want to go this way, we should store a time or something to make sure it only shows right after the update

Copy link
Member

Choose a reason for hiding this comment

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

And if I updated in the meantime to a version higher it will say my update failed

Copy link
Member Author

Choose a reason for hiding this comment

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

All true, which is why I asked about a event

Copy link
Member

Choose a reason for hiding this comment

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

The event will not fix it if the user is not at this page when it is fired?

Copy link
Member Author

Choose a reason for hiding this comment

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

No matter how we do this, it will be some weird situations.
I'll take this part out of the PR, and we can implement something (if needed) when something for home-assistant/supervisor#1946 is in place.

@ludeeus ludeeus changed the title Handle connection drops when upgrading core Handle connection drops when upgrading Sep 2, 2020
@ludeeus ludeeus added the Supervisor Related to the supervisor panel label Sep 2, 2020
@bramkragten bramkragten merged commit 8e506f7 into dev Sep 2, 2020
@bramkragten bramkragten deleted the core-update branch September 2, 2020 14:22
@bramkragten bramkragten mentioned this pull request Sep 4, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed Supervisor Related to the supervisor panel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants