From 8e495870c2de5082cb11d0f20375a6c273f1e7f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke?= Date: Thu, 28 Feb 2019 07:18:48 +0100 Subject: [PATCH] Feature/skill manager error checks (#1981) * Attempt to create skill directory if not existing * Handle missing priority skills * Minor update of comments * Handle skill load exception Make sure an exception while trying to load/reload skill doesn't shutdown thread. * Handle MsmException during SkillManager creation If SkillManager can't be created due to an MsmException wait for network connection and retry. * Update immediately if skill install file is missing Missing skill install file indicates that this is a new venv and the requirements of the skills will need to be reinstalled. * Add basic test for skill_manager Basically only creating the skill_manager but it ensures that msm can be used on all supported python versions --- mycroft/skills/__main__.py | 18 +++++- mycroft/skills/skill_manager.py | 50 +++++++++------ test/unittests/skills/test_skill_manager.py | 69 +++++++++++++++++++++ 3 files changed, 116 insertions(+), 21 deletions(-) create mode 100644 test/unittests/skills/test_skill_manager.py diff --git a/mycroft/skills/__main__.py b/mycroft/skills/__main__.py index fb88a18cd5..f800c0604b 100644 --- a/mycroft/skills/__main__.py +++ b/mycroft/skills/__main__.py @@ -25,8 +25,9 @@ from mycroft.util import ( connected, wait_while_speaking, reset_sigint_handler, create_echo_function, create_daemon, wait_for_exit_signal ) +from mycroft.util.log import LOG -from .skill_manager import SkillManager +from .skill_manager import SkillManager, MsmException from .core import FallbackSkill from .event_scheduler import EventScheduler from .intent_service import IntentService @@ -68,9 +69,20 @@ def _starting_up(): event_scheduler = EventScheduler(bus) # Create a thread that monitors the loaded skills, looking for updates - skill_manager = SkillManager(bus) + try: + skill_manager = SkillManager(bus) + except MsmException: + # skill manager couldn't be created, wait for network connection and + # retry + LOG.info('Msm is uninitialized and requires network connection', + 'to fetch skill information\n' + 'Waiting for network connection...') + while not connected(): + time.sleep(30) + skill_manager = SkillManager(bus) + skill_manager.daemon = True - # Wait until skills have been loaded once before starting to check + # Wait until priority skills have been loaded before checking # network connection skill_manager.load_priority() skill_manager.start() diff --git a/mycroft/skills/skill_manager.py b/mycroft/skills/skill_manager.py index 80bd72e286..cba24a3993 100644 --- a/mycroft/skills/skill_manager.py +++ b/mycroft/skills/skill_manager.py @@ -101,7 +101,9 @@ class SkillManager(Thread): self.update_interval = Configuration.get()['skills']['update_interval'] self.update_interval = int(self.update_interval * 60 * MINUTES) self.dot_msm = join(self.msm.skills_dir, '.msm') - if exists(self.dot_msm): + # Update immediately if the .msm or installed skills file is missing + # otherwise according to timestamp on .msm + if exists(self.dot_msm) and exists(self.installed_skills_file): self.next_download = os.path.getmtime(self.dot_msm) + \ self.update_interval else: @@ -135,6 +137,10 @@ class SkillManager(Thread): repo_config = msm_config['repo'] data_dir = expanduser(config['data_dir']) skills_dir = join(data_dir, msm_config['directory']) + # Try to create the skills directory if it doesn't exist + if not exists(skills_dir): + os.makedirs(skills_dir) + repo_cache = join(data_dir, repo_config['cache']) platform = config['enclosure'].get('platform', 'default') return MycroftSkillsManager( @@ -245,12 +251,15 @@ class SkillManager(Thread): data = {'utterance': dialog.get("skills updated")} self.bus.emit(Message("speak", data)) + # Schedule retry in 5 minutes on failure, after 10 shorter periods + # Go back to 60 minutes wait if default_skill_errored and self.num_install_retries < 10: self.num_install_retries += 1 self.next_download = time.time() + 5 * MINUTES return False self.num_install_retries = 0 + # Update timestamp on .msm file to be used when system is restarted with open(self.dot_msm, 'a'): os.utime(self.dot_msm, None) self.next_download = time.time() + self.update_interval @@ -285,10 +294,7 @@ class SkillManager(Thread): """ skill_path = skill_path.rstrip('/') skill = self.loaded_skills.setdefault(skill_path, {}) - skill.update({ - "id": basename(skill_path), - "path": skill_path - }) + skill.update({"id": basename(skill_path), "path": skill_path}) # check if folder is a skill (must have __init__.py) if not MainModule + ".py" in os.listdir(skill_path): @@ -354,15 +360,19 @@ class SkillManager(Thread): def load_priority(self): skills = {skill.name: skill for skill in self.msm.list()} for skill_name in PRIORITY_SKILLS: - skill = skills[skill_name] - if not skill.is_local: - try: - skill.install() - except Exception: - LOG.exception('Downloading priority skill:' + skill.name) - if not skill.is_local: - continue - self._load_or_reload_skill(skill.path) + skill = skills.get(skill_name) + if skill: + if not skill.is_local: + try: + skill.install() + except Exception: + LOG.exception('Downloading priority skill: ' + '{} failed'.format(skill.name)) + if not skill.is_local: + continue + self._load_or_reload_skill(skill.path) + else: + LOG.error('Priority skill {} can\'t be found') def remove_git_locks(self): """If git gets killed from an abrupt shutdown it leaves lock files""" @@ -392,10 +402,14 @@ class SkillManager(Thread): skill_paths = glob(join(self.msm.skills_dir, '*/')) still_loading = False for skill_path in skill_paths: - still_loading = ( - self._load_or_reload_skill(skill_path) or - still_loading - ) + try: + still_loading = ( + self._load_or_reload_skill(skill_path) or + still_loading + ) + except Exception as e: + LOG.error('(Re)loading of {} failed ({})'.format( + skill_path, repr(e))) if not has_loaded and not still_loading and len(skill_paths) > 0: has_loaded = True self.bus.emit(Message('mycroft.skills.initialized')) diff --git a/test/unittests/skills/test_skill_manager.py b/test/unittests/skills/test_skill_manager.py new file mode 100644 index 0000000000..60604a590a --- /dev/null +++ b/test/unittests/skills/test_skill_manager.py @@ -0,0 +1,69 @@ +import unittest +import mock +import tempfile +from os.path import exists, join +from shutil import rmtree +from test.util import base_config + +from mycroft.configuration import Configuration +from mycroft.skills.skill_manager import SkillManager + +BASE_CONF = base_config() +BASE_CONF['data_dir'] = tempfile.mkdtemp() +BASE_CONF['skills'] = { + 'msm': { + 'directory': 'skills', + 'versioned': True, + 'repo': { + 'cache': '.skills-repo', + 'url': 'https://github.com/MycroftAI/mycroft-skills', + 'branch': '18.08' + } + }, + 'update_interval': 3600, + 'auto_update': False, + 'blacklisted_skills': [], + 'priority_skills': ["mycroft-pairing"] +} + + +class MockEmitter(object): + def __init__(self): + self.reset() + + def emit(self, message): + self.types.append(message.type) + self.results.append(message.data) + + def get_types(self): + return self.types + + def get_results(self): + return self.results + + def on(self, event, f): + pass + + def reset(self): + self.types = [] + self.results = [] + + +class MycroftSkillTest(unittest.TestCase): + emitter = MockEmitter() + + def setUp(self): + self.emitter.reset() + self.temp_dir = tempfile.mkdtemp() + + @mock.patch.dict(Configuration._Configuration__config, BASE_CONF) + def test_create_manager(self): + """ Verify that the skill manager and msm loads as expected and + that the skills dir is created as needed. + """ + SkillManager(self.emitter) + self.assertTrue(exists(join(BASE_CONF['data_dir'], 'skills'))) + + @classmethod + def tearDownClass(cls): + rmtree(BASE_CONF['data_dir'])