-
-
Notifications
You must be signed in to change notification settings - Fork 75
Feature/prevent blocking start async #1303
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
base: main
Are you sure you want to change the base?
Feature/prevent blocking start async #1303
Conversation
…ct the new behavior.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1303 +/- ##
====================================
Coverage 82% 82%
====================================
Files 196 196
Lines 3841 3850 +9
Branches 422 424 +2
====================================
+ Hits 3176 3184 +8
- Misses 500 501 +1
Partials 165 165
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/Runtime/NetDaemon.Runtime/Common/Extensions/HostBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/Runtime/NetDaemon.Runtime/Common/INetDaemonRuntimeInitializedCheck.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, few comments, lets discuss on Discord
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment that we need to discuss further but overall a good implementaiton. Thank you.
@@ -151,12 +164,6 @@ private async Task OnHomeAssistantClientDisconnected(DisconnectReason reason) | |||
logger.LogError(e, "Error disposing applications"); | |||
} | |||
IsConnected = false; | |||
|
|||
if (reason == DisconnectReason.Unauthorized) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code was important since we needed the NetDaemon not to retry connection if it was unauthorized. It should just log and think it is connected. It is a hacky way of doing ti when I see it now but that feature is needed not to make user account locked etc. Unless you handle this somewhere else in the code and I fail to see it.
Proposed change
Made startup behavior non-blocking.
Implemented
INetDaemonRuntimeInitializedCheck
to be able to wait for NetDaemon runtime inititialization.Type of change
Checklist
If user exposed functionality or configuration are added/changed: