-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Feature - check if services ready on boot #2642
Conversation
- skills - audio - speech client - messagebus service - enclosure The main functions now accepts the arguments ready_hook and error_hook allowing a service or runner script to catch these states and perform actions accordingly. This is useful for things like systemd or a desktop launcher. Fix audio service startup
If no watchdog is provided a dummy function will be called
Also asks each service to report back before declaring mycroft.ready
Voight Kampff Integration Test Succeeded (Results) |
The new wait_for_loaded() method will wait until services has been loaded. It has a 3 minute timeout (default) This improves the audio service readiness indication to not trigger until the services are loaded.
Service hooks
Hello @krisgesling! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2020-07-27 10:41:21 UTC |
Also asks each service to report back before declaring mycroft.ready
Voight Kampff Integration Test Succeeded (Results) |
|
||
|
||
def on_ready(): | ||
alive_status.set() |
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.
Since these can be overridden the setting in the alive_status here may not be the best way to do this.
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.
ah good point, maybe I'll shift them to where the hooks are called from.
Does the overall approach make sense though? Should we be moving straight to an external monitoring service or something else?
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.
It sort of makes sense with the other things the enclosure does. I don't have a better suggestion...
I wonder if a separate class for tracking the status and handling the reporting and callbacks would be a good idea. Something along the lines of: forslund@da94167 Each process creates an ProcessStatus object and it handles callbacks and reporting readiness over messagebus |
Interesting! Does this also mean that the ready status is properly reported back into the pairing skill? Because the pairing skill always speaks the paired dialog, but never the wait for ready dialog. |
It should improve things but there's still the case when default skills aren't installed that needs to be handled. But that's probably a separate issue |
Ok, then I think it is best that I create an seperate issue at that repo referencing to this one. |
Closing in favour of #2645 Created in a new PR to maintain discussion and it's context here. |
DO NOT MERGE
Description
Some example code following on from issue #2639
This shifts the
mycroft.ready
message from the Padatious Service to the Enclosure.It sends a request to each service over the message bus to respond if it's alive. As such it assumes that both the bus and the enclosure services are alive as they're required for successfully transmission of the messages.
In it's current state it will always timeout as there's no handler added to the Skills service. I'm sure there's also a better way to handle checking the speech service, and general refactoring is needed.
How to test
Boot Mycroft - should startup, train all intents, then check each service for 2 mins and timeout.
Remove skills service from list to check and it should emit
mycroft.ready
as normal.Questions