Service hooks #2601

forslund · 2020-06-01T16:11:34Z

Description

This adds a simple system of callback hooks to run mycroft as a service or through a single executable. It allows the definition of a wrapper script with handlers for ready states, error states and a watchdog to ensure the system is still running.

Example wrapper script for use with systemd:

import sdnotify

from mycroft.skills.__main__ import main


n = sdnotify.SystemdNotifier()

def on_ready():
    n.notify("READY=1")

def on_stopped():
    n.notify("READY=0")

main(ready_hook=on_ready, stop_hook=on_stopped)

The current watchdog implementations are currently only for the speech client (voice data is received from the mic) and the skills process (skills are updated) due to their natural cyclic structure.

The reason for doing this is to be able to keep startup system requirements outside of core and allowing flexibility. For example these startup hooks can be done for a desktop launcher just as well as for a system service like systemd or finit.

How to test

Create a similar wrapper but instead of sdnotify a simple custom print statement is used in the hooks.

Contributor license agreement signed?

CLA [ Yes ]

devops-mycroft · 2020-06-01T17:00:40Z

Voight Kampff Integration Test Failed (Results)

pep8speaks · 2020-06-01T17:23:36Z

Hello @forslund! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-07-24 20:15:47 UTC

devops-mycroft · 2020-06-01T18:12:34Z

Voight Kampff Integration Test Failed (Results)

j1nx · 2020-06-02T07:10:19Z

Perfect!

I have created systemd files that make use of this wrapper. You can use and test it wih those files;

https://github.com/j1nx/mycroft-systemd

j1nx · 2020-06-06T22:57:21Z

Will watchdog support be added?

forslund · 2020-06-07T21:12:58Z

The skills service and the voice client has watchdogs since those have cyclic sections easily checked for activity, the other services could also get watchdog support in the future but some mechanism would need to be created to make those reliable.

I haven't been able to figure out why the integration tests are failing on Jenkins. Trying to reproduce the issue but haven't succeeded yet. After that the PR might be considered for inclusion.

devops-mycroft · 2020-07-11T07:16:01Z

Voight Kampff Integration Test Failed (Results)

devops-mycroft · 2020-07-17T17:46:39Z

Voight Kampff Integration Test Failed (Results)

devops-mycroft · 2020-07-21T06:37:22Z

Voight Kampff Integration Test Failed (Results)

krisgesling

Hey, just one critical typo and a possible addition. Otherwise looks great -should be really useful!

mycroft/client/enclosure/__main__.py

mycroft/audio/__main__.py

forslund · 2020-07-21T17:11:05Z

That typo explains the vk issues facepalm. I had some reason for leaving out the stopping, but I can't remember it so I'll add one for completeness. Will do this as soon as I get the opportunity.

- 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

forslund · 2020-07-22T09:11:15Z

PR updated and rebased according to comments, hope VK is happy now!

(Sent from a camping chair at N59° 31,487' E015° 02,690')

devops-mycroft · 2020-07-22T09:43:12Z

Voight Kampff Integration Test Succeeded (Results)

j1nx · 2020-07-22T11:33:50Z

Nice progress!

Still hope Watchdog finds it way into the other services as well. Especially the messagebus as everything relies on that service to be available.

forslund · 2020-07-22T12:07:52Z

I will work on adding that in a followup PR, gonna dig into tornado and the ws library and see if it can be achieved in some way...if not perhaps a cyclic message from the messahebus service triggering the wd in the respective clients is a way to add some additional reliability

j1nx · 2020-07-22T12:39:21Z

I remember you where not very keen on adding a cyclic function into mycroft-core, so if the messagebus doesn't have something similar as the voice and skill service, perhaps the other way around?

Send a message to the messagebus from the wrapper and if a proper reply is received, petting the dog...

krisgesling · 2020-07-24T05:34:42Z

I think we'll need to look further into the Audio Service. It's reporting ready before loading any of the services:

2020-07-24 14:52:16.226 | INFO     |  8905 | __main__:<module>:68 | __main__
2020-07-24 14:52:16.227 | INFO     |  8905 | mycroft.messagebus.load_config:load_message_bus_config:33 | Loading message bus configs
2020-07-24 14:52:16.230 | INFO     |  8905 | mycroft.tts.mimic2_tts:__init__:176 | Getting Pre-loaded cache
2020-07-24 14:52:16.236 | INFO     |  8905 | mycroft.tts.mimic2_tts:__init__:178 | Successfully downloaded Pre-loaded cache
2020-07-24 14:52:16.236 | INFO     |  8905 | __main__:main:50 | Starting Audio Services
2020-07-24 14:52:16.237 | INFO     |  8905 | __main__:on_ready:30 | Audio service is ready.
2020-07-24 14:52:16.238 | INFO     |  8905 | mycroft.messagebus.client.client:on_open:114 | Connected
2020-07-24 14:52:16.238 | INFO     |  8905 | mycroft.audio.audioservice:get_services:56 | Loading services from /home/gez/mycroft-core/mycroft/audio/services/
2020-07-24 14:52:16.239 | INFO     |  8905 | mycroft.audio.audioservice:load_services:100 | Loading chromecast
2020-07-24 14:52:21.325 | INFO     |  8905 | mycroft.audio.audioservice:load_services:100 | Loading mopidy
2020-07-24 14:52:21.326 | INFO     |  8905 | mycroft.audio.audioservice:load_services:100 | Loading mplayer
2020-07-24 14:52:21.331 | ERROR    |  8905 | mplayer__init__:<module>:20 | install py_mplayer with pip install git+https://github.com/JarbasAl/py_mplayer
2020-07-24 14:52:21.331 | ERROR    |  8905 | mycroft.audio.audioservice:load_services:106 | Failed to import module mplayer
ModuleNotFoundError("No module named 'py_mplayer'",)
2020-07-24 14:52:21.331 | INFO     |  8905 | mycroft.audio.audioservice:load_services:100 | Loading simple
2020-07-24 14:52:21.333 | INFO     |  8905 | mycroft.audio.audioservice:load_services:100 | Loading vlc
2020-07-24 14:52:21.367 | INFO     |  8905 | mycroft.audio.audioservice:load_services_callback:168 | Finding default backend...
2020-07-24 14:52:21.368 | INFO     |  8905 | mycroft.audio.audioservice:load_services_callback:172 | Found local

My interpretation of ready would be that at least the default service is loaded and can output sound.

The others seem fine.

krisgesling · 2020-07-24T11:10:37Z

I meant to say, don't think my previous comment needs to be addressed in this PR.

forslund · 2020-07-24T17:07:21Z

I can fix that quickly, the audio service loading is done in the background I'll add a status allowing the main function to check if loading has completed.

j1nx · 2020-07-24T19:45:07Z

Gez also meant that the messgaebus watchdog don't need to be addressed in this PR😜😂

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.

forslund · 2020-07-24T20:18:54Z

Updated PR with a commit adding functionality to the audioservice for checking load status and only calling the ready_hook after waiting for services to be loaded.

devops-mycroft · 2020-07-24T20:52:01Z

Voight Kampff Integration Test Succeeded (Results)

krisgesling · 2020-07-27T04:52:51Z

Yeah, this is great!

Thanks again - merging!

devs-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Jun 1, 2020

forslund force-pushed the feature/service-hooks branch from 6cc33ed to ebc01fc Compare June 1, 2020 17:23

forslund force-pushed the feature/service-hooks branch from ebc01fc to 3e6863a Compare June 1, 2020 17:34

forslund force-pushed the feature/service-hooks branch from 3e6863a to 944a236 Compare July 11, 2020 06:28

forslund force-pushed the feature/service-hooks branch from 944a236 to 10e7168 Compare July 17, 2020 17:09

krisgesling mentioned this pull request Jul 21, 2020

Ensure services are fully ready before reporting readiness #2639

Closed

krisgesling suggested changes Jul 21, 2020

View reviewed changes

mycroft/client/enclosure/__main__.py Outdated Show resolved Hide resolved

mycroft/audio/__main__.py Outdated Show resolved Hide resolved

forslund added 3 commits July 22, 2020 10:52

Add support for watchdog in skills and speech client

a1fdae3

If no watchdog is provided a dummy function will be called

Add hook for stopping

236a2ed

forslund force-pushed the feature/service-hooks branch from 10e7168 to 236a2ed Compare July 22, 2020 09:07

Add system for checking audio service readiness

0a04efd

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.

krisgesling approved these changes Jul 27, 2020

View reviewed changes

krisgesling merged commit 273da88 into MycroftAI:dev Jul 27, 2020

forslund mentioned this pull request Aug 14, 2020

Improve readiness check for all services #2648

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Service hooks #2601

Service hooks #2601

forslund commented Jun 1, 2020 •

edited

Loading

devops-mycroft commented Jun 1, 2020

pep8speaks commented Jun 1, 2020 •

edited

Loading

devops-mycroft commented Jun 1, 2020

j1nx commented Jun 2, 2020

j1nx commented Jun 6, 2020

forslund commented Jun 7, 2020

devops-mycroft commented Jul 11, 2020

devops-mycroft commented Jul 17, 2020

devops-mycroft commented Jul 21, 2020

krisgesling left a comment

forslund commented Jul 21, 2020

forslund commented Jul 22, 2020

devops-mycroft commented Jul 22, 2020

j1nx commented Jul 22, 2020

forslund commented Jul 22, 2020 •

edited

Loading

j1nx commented Jul 22, 2020

krisgesling commented Jul 24, 2020

krisgesling commented Jul 24, 2020

forslund commented Jul 24, 2020

j1nx commented Jul 24, 2020 •

edited

Loading

forslund commented Jul 24, 2020

devops-mycroft commented Jul 24, 2020

krisgesling commented Jul 27, 2020

Service hooks #2601

Service hooks #2601

Conversation

forslund commented Jun 1, 2020 • edited Loading

Description

How to test

Contributor license agreement signed?

devops-mycroft commented Jun 1, 2020

pep8speaks commented Jun 1, 2020 • edited Loading

Comment last updated at 2020-07-24 20:15:47 UTC

devops-mycroft commented Jun 1, 2020

j1nx commented Jun 2, 2020

j1nx commented Jun 6, 2020

forslund commented Jun 7, 2020

devops-mycroft commented Jul 11, 2020

devops-mycroft commented Jul 17, 2020

devops-mycroft commented Jul 21, 2020

krisgesling left a comment

Choose a reason for hiding this comment

forslund commented Jul 21, 2020

forslund commented Jul 22, 2020

devops-mycroft commented Jul 22, 2020

j1nx commented Jul 22, 2020

forslund commented Jul 22, 2020 • edited Loading

j1nx commented Jul 22, 2020

krisgesling commented Jul 24, 2020

krisgesling commented Jul 24, 2020

forslund commented Jul 24, 2020

j1nx commented Jul 24, 2020 • edited Loading

forslund commented Jul 24, 2020

devops-mycroft commented Jul 24, 2020

krisgesling commented Jul 27, 2020

forslund commented Jun 1, 2020 •

edited

Loading

pep8speaks commented Jun 1, 2020 •

edited

Loading

forslund commented Jul 22, 2020 •

edited

Loading

j1nx commented Jul 24, 2020 •

edited

Loading