Hello community,
here is the log from the commit of package python-msm for openSUSE:Factory
checked in at 2019-09-04 09:10:09
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Comparing /work/SRC/openSUSE:Factory/python-msm (Old)
and /work/SRC/openSUSE:Factory/.python-msm.new.7948 (New)
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "python-msm"
Wed Sep 4 09:10:09 2019 rev:10 rq:727100 version:0.8.2
Changes:
--------
--- /work/SRC/openSUSE:Factory/python-msm/python-msm.changes 2019-07-24
20:37:15.490562744 +0200
+++ /work/SRC/openSUSE:Factory/.python-msm.new.7948/python-msm.changes
2019-09-04 09:10:22.714979546 +0200
@@ -1,0 +2,12 @@
+Tue Aug 27 12:44:58 UTC 2019 - Marketa Calabkova <mcalabk...@suse.com>
+
+- update to version 0.8.2
+ * Add new platforms
+ * Remove temporary copy of skill after action
+ * added a skill_list property to cache the results of the list() method
+ * added tests for more coverage
+ * Fix infinite recursion issue
+ * Make from_folder use msm skill cache if possible
+- reapplied add-local-patch-support.patch
+
+-------------------------------------------------------------------
Old:
----
v0.7.8.tar.gz
New:
----
v0.8.2.tar.gz
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Other differences:
------------------
++++++ python-msm.spec ++++++
--- /var/tmp/diff_new_pack.IifMCX/_old 2019-09-04 09:10:29.786978563 +0200
+++ /var/tmp/diff_new_pack.IifMCX/_new 2019-09-04 09:10:29.790978563 +0200
@@ -19,7 +19,7 @@
%define skip_python2 1
%{?!python_module:%define python_module() python-%{**} python3-%{**}}
Name: python-msm
-Version: 0.7.8
+Version: 0.8.2
Release: 0
Summary: Mycroft Skills Manager
License: Apache-2.0
++++++ add-local-patch-support.patch ++++++
--- /var/tmp/diff_new_pack.IifMCX/_old 2019-09-04 09:10:29.822978558 +0200
+++ /var/tmp/diff_new_pack.IifMCX/_new 2019-09-04 09:10:29.822978558 +0200
@@ -1,17 +1,17 @@
-Index: mycroft-skills-manager-0.7.6/msm/skill_entry.py
+Index: mycroft-skills-manager-0.8.0/msm/skill_entry.py
===================================================================
---- mycroft-skills-manager-0.7.6.orig/msm/skill_entry.py
-+++ mycroft-skills-manager-0.7.6/msm/skill_entry.py
+--- mycroft-skills-manager-0.8.0.orig/msm/skill_entry.py
++++ mycroft-skills-manager-0.8.0/msm/skill_entry.py
@@ -45,6 +45,8 @@ from msm.exceptions import PipRequiremen
SystemRequirementsException, AlreadyInstalled, SkillModified, \
AlreadyRemoved, RemoveException, CloneException, NotInstalled,
GitException
- from msm.util import Git
+ from msm.util import cached_property, Git
+from msm.local_patches_utils import apply_skill_patch, \
+ reverse_skill_patch, remove_applied_skill_patch
LOG = logging.getLogger(__name__)
-@@ -420,6 +422,7 @@ class SkillEntry(object):
+@@ -430,6 +432,7 @@ class SkillEntry(object):
raise AlreadyRemoved(self.name)
try:
rmtree(self.path)
@@ -19,7 +19,7 @@
self.is_local = False
except OSError as e:
raise RemoveException(str(e))
-@@ -447,6 +450,7 @@ class SkillEntry(object):
+@@ -457,6 +460,7 @@ class SkillEntry(object):
try:
move(tmp_location, self.path)
@@ -27,7 +27,7 @@
if self.msm:
self.run_skill_requirements()
self.install_system_deps()
-@@ -484,6 +488,7 @@ class SkillEntry(object):
+@@ -494,6 +498,7 @@ class SkillEntry(object):
with git_to_msm_exceptions():
sha_before = git.rev_parse('HEAD')
@@ -35,7 +35,7 @@
modified_files = git.status(porcelain=True, untracked='no')
if modified_files != '':
raise SkillModified('Uncommitted changes:\n' + modified_files)
-@@ -495,6 +500,7 @@ class SkillEntry(object):
+@@ -505,6 +510,7 @@ class SkillEntry(object):
git.checkout(self._find_sha_branch())
git.merge(self.sha or 'origin/HEAD', ff_only=True)
@@ -43,10 +43,10 @@
sha_after = git.rev_parse('HEAD')
-Index: mycroft-skills-manager-0.7.6/msm/local_patches_utils.py
+Index: mycroft-skills-manager-0.8.0/msm/local_patches_utils.py
===================================================================
--- /dev/null
-+++ mycroft-skills-manager-0.7.6/msm/local_patches_utils.py
++++ mycroft-skills-manager-0.8.0/msm/local_patches_utils.py
@@ -0,0 +1,99 @@
+# Copyright (c) 2018 Mycroft AI, Inc.
+#
++++++ v0.7.8.tar.gz -> v0.8.2.tar.gz ++++++
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn'
'--exclude=.svnignore'
old/mycroft-skills-manager-0.7.8/msm/mycroft_skills_manager.py
new/mycroft-skills-manager-0.8.2/msm/mycroft_skills_manager.py
--- old/mycroft-skills-manager-0.7.8/msm/mycroft_skills_manager.py
2019-07-09 16:28:02.000000000 +0200
+++ new/mycroft-skills-manager-0.8.2/msm/mycroft_skills_manager.py
2019-08-23 12:31:42.000000000 +0200
@@ -19,31 +19,51 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-import time
+"""Install, remove, update and track the skills on a device
+MSM can be used on the command line but is also used by Mycroft core daemons.
+"""
+import time
import logging
from functools import wraps
from glob import glob
from multiprocessing.pool import ThreadPool
-from os.path import expanduser, join, dirname, isdir
+from os import path
from typing import Dict, List
from msm import GitException
-from msm.exceptions import (MsmException, SkillNotFound, MultipleSkillMatches,
- AlreadyInstalled)
+from msm.exceptions import (
+ AlreadyInstalled,
+ AlreadyRemoved,
+ MsmException,
+ MultipleSkillMatches,
+ RemoveException,
+ SkillNotFound
+)
from msm.skill_entry import SkillEntry
from msm.skill_repo import SkillRepo
-from msm.skills_data import (build_skill_entry, get_skill_entry,
- write_skills_data, load_skills_data,
- skills_data_hash)
-from msm.util import MsmProcessLock
+from msm.skill_state import (
+ initialize_skill_state,
+ get_skill_state,
+ write_device_skill_state,
+ load_device_skill_state,
+ device_skill_state_hash
+)
+from msm.util import cached_property, MsmProcessLock
LOG = logging.getLogger(__name__)
CURRENT_SKILLS_DATA_VERSION = 2
+ONE_DAY = 86400
-def save_skills_data(func):
+def save_device_skill_state(func):
+ """Decorator to overwrite the skills.json file when skill state changes.
+
+ The methods decorated with this function are executed in threads. So,
+ this contains some funky logic to keep the threads from stepping on one
+ another.
+ """
@wraps(func)
def func_wrapper(self, *args, **kwargs):
will_save = False
@@ -53,7 +73,7 @@
ret = func(self, *args, **kwargs)
# Write only if no exception occurs
if will_save:
- self.write_skills_data()
+ self.write_device_skill_state()
finally:
# Always restore saving_handled flag
if will_save:
@@ -65,195 +85,352 @@
class MycroftSkillsManager(object):
- SKILL_GROUPS = {'default', 'mycroft_mark_1', 'picroft', 'kde'}
+ SKILL_GROUPS = {'default', 'mycroft_mark_1', 'picroft', 'kde',
+ 'respeaker', 'mycroft_mark_2', 'mycroft_mark_2pi'}
DEFAULT_SKILLS_DIR = "/opt/mycroft/skills"
def __init__(self, platform='default', skills_dir=None, repo=None,
versioned=True):
self.platform = platform
self.skills_dir = (
- expanduser(skills_dir or '') or self.DEFAULT_SKILLS_DIR
+ path.expanduser(skills_dir or '') or self.DEFAULT_SKILLS_DIR
)
self.repo = repo or SkillRepo()
self.versioned = versioned
self.lock = MsmProcessLock()
- self.skills_data = None
+ # Property placeholders
+ self._all_skills = None
+ self._default_skills = None
+ self._local_skills = None
+ self._device_skill_state = None
+
self.saving_handled = False
- self.skills_data_hash = ''
+ self.device_skill_state_hash = ''
with self.lock:
- self.sync_skills_data()
+ self._init_skills_data()
+
+ @cached_property(ttl=ONE_DAY)
+ def all_skills(self):
+ """Getting a list of skills can take a while so cache it.
+
+ The list method is called several times in this class and in core.
+ Skill data on a device just doesn't change that frequently so
+ getting a fresh list that many times does not make a lot of sense.
+ The cache will expire every hour to pick up any changes in the
+ mycroft-skills repo.
+
+ Skill installs and updates will invalidate the cache, which will
+ cause this property to refresh next time is is referenced.
+
+ The list method can be called directly if a fresh skill list is needed.
+ """
+ if self._all_skills is None:
+ self._all_skills = self._get_all_skills()
+
+ return self._all_skills
+
+ def _get_all_skills(self):
+ LOG.info('building SkillEntry objects for all skills')
+ self._refresh_skill_repo()
+ remote_skills = self._get_remote_skills()
+ all_skills = self._merge_remote_with_local(remote_skills)
+
+ return all_skills
+
+ def list(self):
+ """Load a list of SkillEntry objects from both local and remote skills
- def __upgrade_skills_data(self, skills_data):
- local_skills = [s for s in self.list() if s.is_local]
- if skills_data.get('version', 0) == 0:
- skills_data = self.__upgrade_to_v1(skills_data, local_skills)
- if skills_data['version'] == 1:
- skills_data = self.__upgrade_to_v2(skills_data, local_skills)
- return skills_data
-
- def __upgrade_to_v1(self, skills_data, local_skills):
- new = {
- 'blacklist': [],
- 'version': 1,
- 'skills': []
- }
- default_skills = [s.name for s in self.list_defaults()]
- for skill in local_skills:
- if 'origin' in skills_data.get(skill.name, {}):
- origin = skills_data[skill.name]['origin']
- elif skill.name in default_skills:
- origin = 'default'
- elif skill.url:
- origin = 'cli'
+ It is necessary to load both local and remote skills at
+ the same time to correctly associate local skills with the name
+ in the repo and remote skills with any custom path that they
+ have been downloaded to.
+
+ The return value of this function is cached in the all_skills property.
+ Only call this method if you need a fresh version of the SkillEntry
+ objects.
+ """
+ all_skills = self._get_all_skills()
+ self._invalidate_skills_cache(new_value=all_skills)
+
+ return all_skills
+
+ def _refresh_skill_repo(self):
+ """Get the latest mycroft-skills repo code."""
+ try:
+ self.repo.update()
+ except GitException as e:
+ if not path.isdir(self.repo.path):
+ raise
+ LOG.warning('Failed to update repo: {}'.format(repr(e)))
+
+ def _get_remote_skills(self):
+ """Build a dictionary of skills in mycroft-skills repo keyed by id"""
+ remote_skills = []
+ for name, _, url, sha in self.repo.get_skill_data():
+ skill_dir = SkillEntry.create_path(self.skills_dir, url, name)
+ sha = sha if self.versioned else ''
+ remote_skills.append(
+ SkillEntry(name, skill_dir, url, sha, msm=self)
+ )
+
+ return {skill.id: skill for skill in remote_skills}
+
+ def _merge_remote_with_local(self, remote_skills):
+ """Merge the skills found in the repo with those installed locally."""
+ all_skills = []
+ for skill_file in glob(path.join(self.skills_dir, '*', '__init__.py')):
+ skill = SkillEntry.from_folder(path.dirname(skill_file), msm=self,
+ use_cache=False)
+ if skill.id in remote_skills:
+ skill.attach(remote_skills.pop(skill.id))
+ all_skills.append(skill)
+ all_skills.extend(remote_skills.values())
+
+ return all_skills
+
+ @property
+ def local_skills(self):
+ """Property containing a dictionary of local skills keyed by name."""
+ if self._local_skills is None:
+ self._local_skills = {
+ s.name: s for s in self.all_skills if s.is_local
+ }
+
+ return self._local_skills
+
+ @property
+ def default_skills(self):
+ if self._default_skills is None:
+ default_skill_groups = self.list_all_defaults()
+ try:
+ default_skill_group = default_skill_groups[self.platform]
+ except KeyError:
+ LOG.error(
+ 'No default skill list found for platform "{}". '
+ 'Using base list.'.format(self.platform)
+ )
+ default_skill_group = default_skill_groups.get('default', [])
+ self._default_skills = {s.name: s for s in default_skill_group}
+
+ return self._default_skills
+
+ def list_all_defaults(self): # type: () -> Dict[str, List[SkillEntry]]
+ """Generate dictionary of default skills in all default skill groups"""
+ all_skills = {skill.name: skill for skill in self.all_skills}
+ default_skills = {group: [] for group in self.SKILL_GROUPS}
+
+ for group_name, skill_names in self.repo.get_default_skill_names():
+ group_skills = []
+ for skill_name in skill_names:
+ try:
+ group_skills.append(all_skills[skill_name])
+ except KeyError:
+ LOG.warning('No such default skill: ' + skill_name)
+ default_skills[group_name] = group_skills
+
+ return default_skills
+
+ def _init_skills_data(self):
+ """Initial load of the skill state that occurs upon instantiation.
+
+ If the skills state was upgraded after it was loaded, write the
+ updated skills state to disk.
+ """
+ try:
+ del(self.device_skill_state['upgraded'])
+ except KeyError:
+ self.device_skill_state_hash = device_skill_state_hash(
+ self.device_skill_state
+ )
+ else:
+ self.write_device_skill_state()
+
+ @property
+ def device_skill_state(self):
+ """Dictionary representing the state of skills on a device."""
+ if self._device_skill_state is None:
+ self._device_skill_state = load_device_skill_state()
+ skills_data_version = self._device_skill_state.get('version', 0)
+ if skills_data_version < CURRENT_SKILLS_DATA_VERSION:
+ self._upgrade_skills_data()
else:
- origin = 'non-msm'
- beta = skills_data.get(skill.name, {}).get('beta', False)
- entry = build_skill_entry(skill.name, origin, beta,
- skill.skill_gid)
- entry['installed'] = \
- skills_data.get(skill.name, {}).get('installed') or 0
- if isinstance(entry['installed'], bool):
- entry['installed'] = 0
-
- entry['update'] = \
- skills_data.get(skill.name, {}).get('updated') or 0
-
- new['skills'].append(entry)
- new['upgraded'] = True
- return new
+ self._sync_device_skill_state()
+
+ return self._device_skill_state
+
+ def _upgrade_skills_data(self):
+ """Upgrade the contents of the device skills state if needed."""
+ if self._device_skill_state.get('version', 0) == 0:
+ self._upgrade_to_v1()
+ if self._device_skill_state['version'] == 1:
+ self._upgrade_to_v2()
+
+ def _upgrade_to_v1(self):
+ """Upgrade the device skills state to version one."""
+ self._device_skill_state.update(blacklist=[], version=1, skills=[])
+ for skill in self.local_skills.values():
+ skill_data = self._device_skill_state.get(skill.name, {})
+ try:
+ origin = skill_data['origin']
+ except KeyError:
+ origin = self._determine_skill_origin(skill)
+ beta = skill_data.get('beta', False)
+ skill_state = initialize_skill_state(
+ skill.name,
+ origin,
+ beta,
+ skill.skill_gid
+ )
+ skill_state['installed'] = skill_data.get('installed', 0)
+ if isinstance(skill_state['installed'], bool):
+ skill_state['installed'] = 0
+ skill_state['updated'] = skill_data.get('updated', 0)
+ self._device_skill_state['skills'].append(skill_state)
+ self._device_skill_state.update(upgraded=True)
- def __upgrade_to_v2(self, skills_data, local_skills):
- """ Upgrade to v2 of the skills.json format.
+ def _upgrade_to_v2(self):
+ """Upgrade the device skills state to version 2.
This adds the skill_gid field to skill entries.
"""
- local_skill_dict = {s.name: s for s in local_skills}
+ self._update_skill_gid()
+ self._device_skill_state.update(version=2, upgraded=True)
- for s in skills_data['skills']:
- if s['name'] in local_skill_dict:
- skill_info = local_skill_dict[s['name']]
- s['skill_gid'] = skill_info.skill_gid
+ def _sync_device_skill_state(self):
+ """Sync device's skill state with with actual skills on disk."""
+ self._add_skills_to_state()
+ self._remove_skills_from_state()
+ self._update_skill_gid()
+
+ def _add_skills_to_state(self):
+ """Add local skill to state if it is not already there."""
+ skill_names = [s['name'] for s in self._device_skill_state['skills']]
+ for skill in self.local_skills.values():
+ if skill.name not in skill_names:
+ origin = self._determine_skill_origin(skill)
+ skill_state = initialize_skill_state(
+ skill.name,
+ origin,
+ False,
+ skill.skill_gid
+ )
+ self._device_skill_state['skills'].append(skill_state)
+
+ def _remove_skills_from_state(self):
+ """Remove skills from state that no longer exist in the filesystem."""
+ skills_to_remove = []
+ for skill in self._device_skill_state['skills']:
+ is_not_local = skill['name'] not in self.local_skills
+ is_installed_state = skill['installation'] == 'installed'
+ if is_not_local and is_installed_state:
+ skills_to_remove.append(skill)
+
+ for skill in skills_to_remove:
+ self._device_skill_state['skills'].remove(skill)
+
+ def _update_skill_gid(self):
+ for skill in self._device_skill_state['skills']:
+ try:
+ local_skill = self.local_skills[skill['name']]
+ except KeyError:
+ skill['skill_gid'] = ''
else:
- s['skill_gid'] = ''
- skills_data['version'] = 2
- return skills_data
-
- def curate_skills_data(self, skills_data):
- """ Sync skills_data with actual skills on disk. """
- local_skills = [s for s in self.list() if s.is_local]
- default_skills = [s.name for s in self.list_defaults()]
- local_skill_names = [s.name for s in local_skills]
- skills_data_skills = [s['name'] for s in skills_data['skills']]
-
- # Check for skills that aren't in the list
- for skill in local_skills:
- if skill.name not in skills_data_skills:
- if skill.name in default_skills:
- origin = 'default'
- elif skill.url:
- origin = 'cli'
- else:
- origin = 'non-msm'
- entry = build_skill_entry(skill.name, origin, False,
- skill.skill_gid)
- skills_data['skills'].append(entry)
-
- # Check for skills in the list that doesn't exist in the filesystem
- remove_list = []
- for s in skills_data.get('skills', []):
- if (s['name'] not in local_skill_names and
- s['installation'] == 'installed'):
- remove_list.append(s)
- for skill in remove_list:
- skills_data['skills'].remove(skill)
-
- # Update skill gids
- for s in local_skills:
- for e in skills_data['skills']:
- if e['name'] == s.name:
- e['skill_gid'] = s.skill_gid
-
- return skills_data
-
- def load_skills_data(self) -> dict:
- skills_data = load_skills_data()
- if skills_data.get('version', 0) < CURRENT_SKILLS_DATA_VERSION:
- skills_data = self.__upgrade_skills_data(skills_data)
- else:
- skills_data = self.curate_skills_data(skills_data)
- return skills_data
+ skill['skill_gid'] = local_skill.skill_gid
- def sync_skills_data(self):
- """ Update internal skill_data_structure from disk. """
- self.skills_data = self.load_skills_data()
- if 'upgraded' in self.skills_data:
- self.skills_data.pop('upgraded')
+ def _determine_skill_origin(self, skill):
+ if skill.name in self.default_skills:
+ origin = 'default'
+ elif skill.url:
+ origin = 'cli'
else:
- self.skills_data_hash = skills_data_hash(self.skills_data)
+ origin = 'non-msm'
+
+ return origin
- def write_skills_data(self, data=None):
- """ Write skills data hash if it has been modified. """
- data = data or self.skills_data
- if skills_data_hash(data) != self.skills_data_hash:
- write_skills_data(data)
- self.skills_data_hash = skills_data_hash(data)
+ def write_device_skill_state(self, data=None):
+ """Write device's skill state to disk if it has been modified."""
+ data = data or self.device_skill_state
+ if device_skill_state_hash(data) != self.device_skill_state_hash:
+ write_device_skill_state(data)
+ self.device_skill_state_hash = device_skill_state_hash(data)
- @save_skills_data
+ @save_device_skill_state
def install(self, param, author=None, constraints=None, origin=''):
"""Install by url or name"""
if isinstance(param, SkillEntry):
skill = param
else:
skill = self.find_skill(param, author)
- entry = build_skill_entry(skill.name, origin, skill.is_beta,
- skill.skill_gid)
+ skill_state = initialize_skill_state(
+ skill.name,
+ origin,
+ skill.is_beta,
+ skill.skill_gid
+ )
try:
skill.install(constraints)
- entry['installed'] = time.time()
- entry['installation'] = 'installed'
- entry['status'] = 'active'
- entry['beta'] = skill.is_beta
except AlreadyInstalled:
- entry = None
+ log_msg = 'Skill {} already installed - ignoring install request'
+ LOG.info(log_msg.format(skill.name))
+ skill_state = None
raise
except MsmException as e:
- entry['installation'] = 'failed'
- entry['status'] = 'error'
- entry['failure_message'] = repr(e)
- raise
+ LOG.exception('Failed to install skill ' + skill.name)
+ skill_state.update(
+ installation='failed',
+ status='error',
+ failure_message=str(e)
+ )
+ else:
+ skill_state.update(
+ installed=time.time(),
+ installation='installed',
+ status='active',
+ beta=skill.is_beta
+ )
finally:
# Store the entry in the list
- if entry:
- self.skills_data['skills'].append(entry)
+ if skill_state is not None:
+ self.device_skill_state['skills'].append(skill_state)
+ self._invalidate_skills_cache()
- @save_skills_data
+ @save_device_skill_state
def remove(self, param, author=None):
"""Remove by url or name"""
if isinstance(param, SkillEntry):
skill = param
else:
skill = self.find_skill(param, author)
- skill.remove()
- skills = [s for s in self.skills_data['skills']
- if s['name'] != skill.name]
- self.skills_data['skills'] = skills
- return
+ try:
+ skill.remove()
+ except AlreadyRemoved:
+ LOG.info('Skill {} has already been removed'.format(skill.name))
+ except RemoveException:
+ LOG.exception('Failed to remove skill ' + skill.name)
+ raise
+ else:
+ remaining_skills = []
+ for skill_state in self.device_skill_state['skills']:
+ if skill_state['name'] != skill.name:
+ remaining_skills.append(skill_state)
+ self.device_skill_state['skills'] = remaining_skills
+ self._invalidate_skills_cache()
def update_all(self):
- local_skills = [skill for skill in self.list() if skill.is_local]
-
def update_skill(skill):
- entry = get_skill_entry(skill.name, self.skills_data)
+ entry = get_skill_state(skill.name, self.device_skill_state)
if entry:
entry['beta'] = skill.is_beta
if skill.update():
+ self._invalidate_skills_cache()
+ self._device_skill_state = None
if entry:
entry['updated'] = time.time()
- return self.apply(update_skill, local_skills)
+ return self.apply(update_skill, self.local_skills.values())
- @save_skills_data
+ @save_device_skill_state
def update(self, skill=None, author=None):
"""Update all downloaded skills or one specified skill."""
if skill is None:
@@ -261,15 +438,16 @@
else:
if isinstance(skill, str):
skill = self.find_skill(skill, author)
- entry = get_skill_entry(skill.name, self.skills_data)
- if entry:
- entry['beta'] = skill.is_beta
+ skill_state = get_skill_state(skill.name, self.device_skill_state)
+ if skill_state:
+ skill_state['beta'] = skill.is_beta
if skill.update():
# On successful update update the update value
- if entry:
- entry['updated'] = time.time()
+ if skill_state:
+ skill_state['updated'] = time.time()
+ self._invalidate_skills_cache()
- @save_skills_data
+ @save_device_skill_state
def apply(self, func, skills, max_threads=20):
"""Run a function on all skills in parallel"""
@@ -290,7 +468,7 @@
with ThreadPool(max_threads) as tp:
return tp.map(run_item, skills)
- @save_skills_data
+ @save_device_skill_state
def install_defaults(self):
"""Installs the default skills, updates all others"""
@@ -300,83 +478,40 @@
else:
self.install(skill, origin='default')
- return self.apply(install_or_update_skill, self.list_defaults())
-
- def list_all_defaults(self): # type: () -> Dict[str, List[SkillEntry]]
- """Returns {'skill_group': [SkillEntry('name')]}"""
- skills = self.list()
- name_to_skill = {skill.name: skill for skill in skills}
- defaults = {group: [] for group in self.SKILL_GROUPS}
-
- for section_name, skill_names in self.repo.get_default_skill_names():
- section_skills = []
- for skill_name in skill_names:
- if skill_name in name_to_skill:
- section_skills.append(name_to_skill[skill_name])
- else:
- LOG.warning('No such default skill: ' + skill_name)
- defaults[section_name] = section_skills
-
- return defaults
-
- def list_defaults(self):
- skill_groups = self.list_all_defaults()
-
- if self.platform not in skill_groups:
- LOG.error('Unknown platform:' + self.platform)
- return skill_groups.get(self.platform,
- skill_groups.get('default', []))
+ return self.apply(
+ install_or_update_skill,
+ self.default_skills.values()
+ )
- def list(self):
- """
- Load a list of SkillEntry objects from both local and
- remote skills
+ def _invalidate_skills_cache(self, new_value=None):
+ """Reset the cached skill lists in case something changed.
- It is necessary to load both local and remote skills at
- the same time to correctly associate local skills with the name
- in the repo and remote skills with any custom path that they
- have been downloaded to
+ The cached_property decorator builds a _cache instance attribute
+ storing a dictionary of cached values. Deleting from this attribute
+ invalidates the cache.
"""
- try:
- self.repo.update()
- except GitException as e:
- if not isdir(self.repo.path):
- raise
- LOG.warning('Failed to update repo: {}'.format(repr(e)))
- remote_skill_list = (
- SkillEntry(
- name, SkillEntry.create_path(self.skills_dir, url, name),
- url, sha if self.versioned else '', msm=self
- )
- for name, path, url, sha in self.repo.get_skill_data()
- )
- remote_skills = {
- skill.id: skill for skill in remote_skill_list
- }
- all_skills = []
- for skill_file in glob(join(self.skills_dir, '*', '__init__.py')):
- skill = SkillEntry.from_folder(dirname(skill_file), msm=self)
- if skill.id in remote_skills:
- skill.attach(remote_skills.pop(skill.id))
- all_skills.append(skill)
- all_skills += list(remote_skills.values())
- return all_skills
+ LOG.info('invalidating skills cache')
+ if hasattr(self, '_cache'):
+ del self._cache['all_skills']
+ self._all_skills = None if new_value is None else new_value
+ self._local_skills = None
+ self._default_skills = None
def find_skill(self, param, author=None, skills=None):
# type: (str, str, List[SkillEntry]) -> SkillEntry
"""Find skill by name or url"""
if param.startswith('https://') or param.startswith('http://'):
repo_id = SkillEntry.extract_repo_id(param)
- for skill in self.list():
+ for skill in self.all_skills:
if skill.id == repo_id:
return skill
name = SkillEntry.extract_repo_name(param)
- path = SkillEntry.create_path(self.skills_dir, param)
- return SkillEntry(name, path, param, msm=self)
+ skill_directory = SkillEntry.create_path(self.skills_dir, param)
+ return SkillEntry(name, skill_directory, param, msm=self)
else:
skill_confs = {
skill: skill.match(param, author)
- for skill in skills or self.list()
+ for skill in skills or self.all_skills
}
best_skill, score = max(skill_confs.items(), key=lambda x: x[1])
LOG.info('Best match ({}): {} by {}'.format(
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn'
'--exclude=.svnignore' old/mycroft-skills-manager-0.7.8/msm/skill_entry.py
new/mycroft-skills-manager-0.8.2/msm/skill_entry.py
--- old/mycroft-skills-manager-0.7.8/msm/skill_entry.py 2019-07-09
16:28:02.000000000 +0200
+++ new/mycroft-skills-manager-0.8.2/msm/skill_entry.py 2019-08-23
12:31:42.000000000 +0200
@@ -44,7 +44,7 @@
from msm.exceptions import PipRequirementsException, \
SystemRequirementsException, AlreadyInstalled, SkillModified, \
AlreadyRemoved, RemoveException, CloneException, NotInstalled, GitException
-from msm.util import Git
+from msm.util import cached_property, Git
LOG = logging.getLogger(__name__)
@@ -54,6 +54,7 @@
# default constraints to use if no are given
DEFAULT_CONSTRAINTS = '/etc/mycroft/constraints.txt'
+FIVE_MINUTES = 300
@contextmanager
@@ -92,6 +93,10 @@
shutil.copytree(self.old_path, self.path)
self.is_local = exists(self.path)
raise
+ finally:
+ # Remove temporary path if needed
+ if self.old_path and exists(self.old_path):
+ rmtree(self.old_path)
return wrapper
@@ -137,10 +142,10 @@
@property
def is_dirty(self):
- """ True if different from the version in the mycroft-skills repo.
+ """True if different from the version in the mycroft-skills repo.
Considers a skill dirty if
- - the checkedout sha doesn't match the mycroft-skills repo
+ - the checkout sha doesn't match the mycroft-skills repo
- the skill doesn't exist in the mycroft-skills repo
- the skill is not a git repo
- has local modifications
@@ -151,7 +156,7 @@
checkout = Git(self.path)
mod = checkout.status(porcelain=True, untracked_files='no') != ''
current_sha = checkout.rev_parse('HEAD')
- except GitCommandError: # Not a git checkout
+ except GitCommandError: # Not a git checkout
return True
skill_shas = {d[0]: d[3] for d in self.msm.repo.get_skill_data()}
@@ -159,9 +164,16 @@
current_sha != skill_shas[self.name] or
mod)
- @property
+ @cached_property(ttl=FIVE_MINUTES)
def skill_gid(self):
- """ Format skill gid for the skill. """
+ """Format skill gid for the skill.
+
+ This property does some Git gymnastics to determine its return value.
+ When a device boots, each skill accesses this property several times.
+ To reduce the amount of boot time, cache the value returned by this
+ property. Cache expires five minutes after it is generated.
+ """
+ LOG.debug('Generating skill_gid for ' + self.name)
gid = ''
if self.is_dirty:
gid += '@|'
@@ -184,7 +196,19 @@
return self
@classmethod
- def from_folder(cls, path, msm=None):
+ def from_folder(cls, path, msm=None, use_cache=True):
+ """Find or create skill entry from folder path.
+
+ Arguments:
+ path: path of skill folder
+ msm: msm instance to use for caching and extended
information
+ retrieval.
+ use_cache: Enable/Disable cache usage. defaults to True
+ """
+ if msm and use_cache:
+ skills = {skill.path: skill for skill in msm.local_skills.values()}
+ if path in skills:
+ return skills[path]
return cls(None, path, cls.find_git_url(path), msm=msm)
@classmethod
@@ -508,6 +532,10 @@
def find_git_url(path):
"""Get the git url from a folder"""
try:
+ LOG.debug(
+ 'Attempting to retrieve the remote origin URL config for '
+ 'skill in path ' + path
+ )
return Git(path).config('remote.origin.url')
except GitError:
return ''
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn'
'--exclude=.svnignore' old/mycroft-skills-manager-0.7.8/msm/skill_state.py
new/mycroft-skills-manager-0.8.2/msm/skill_state.py
--- old/mycroft-skills-manager-0.7.8/msm/skill_state.py 1970-01-01
01:00:00.000000000 +0100
+++ new/mycroft-skills-manager-0.8.2/msm/skill_state.py 2019-08-23
12:31:42.000000000 +0200
@@ -0,0 +1,65 @@
+"""Functions related to manipulating the skills.json file."""
+import json
+from logging import getLogger
+from os.path import expanduser, isfile
+
+LOG = getLogger(__name__)
+SKILL_STATE_PATH = '~/.mycroft/skills.json'
+
+
+def load_device_skill_state() -> dict:
+ """Contains info on how skills should be updated"""
+ skills_data_path = expanduser(SKILL_STATE_PATH)
+ device_skill_state = {}
+ if isfile(skills_data_path):
+ try:
+ with open(skills_data_path) as skill_state_file:
+ device_skill_state = json.load(skill_state_file)
+ except json.JSONDecodeError:
+ LOG.exception('failed to load skills.json')
+
+ return device_skill_state
+
+
+def write_device_skill_state(data: dict):
+ """Write the device skill state to disk."""
+ skill_state_path = expanduser(SKILL_STATE_PATH)
+ with open(skill_state_path, 'w') as skill_state_file:
+ json.dump(data, skill_state_file, indent=4, separators=(',', ':'))
+
+
+def get_skill_state(name, device_skill_state) -> dict:
+ """Find a skill entry in the device skill state and returns it."""
+ skill_state_return = {}
+ for skill_state in device_skill_state.get('skills', []):
+ if skill_state.get('name') == name:
+ skill_state_return = skill_state
+
+ return skill_state_return
+
+
+def initialize_skill_state(name, origin, beta, skill_gid) -> dict:
+ """Create a new skill entry
+
+ Arguments:
+ name: skill name
+ origin: the source of the installation
+ beta: Boolean indicating wether the skill is in beta
+ skill_gid: skill global id
+ Returns:
+ populated skills entry
+ """
+ return dict(
+ name=name,
+ origin=origin,
+ beta=beta,
+ status='active',
+ installed=0,
+ updated=0,
+ installation='installed',
+ skill_gid=skill_gid
+ )
+
+
+def device_skill_state_hash(data):
+ return hash(json.dumps(data, sort_keys=True))
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn'
'--exclude=.svnignore' old/mycroft-skills-manager-0.7.8/msm/skills_data.py
new/mycroft-skills-manager-0.8.2/msm/skills_data.py
--- old/mycroft-skills-manager-0.7.8/msm/skills_data.py 2019-07-09
16:28:02.000000000 +0200
+++ new/mycroft-skills-manager-0.8.2/msm/skills_data.py 1970-01-01
01:00:00.000000000 +0100
@@ -1,60 +0,0 @@
-"""
- Functions related to manipulating the skills_data.json
-"""
-
-import json
-from os.path import expanduser, isfile
-
-
-def load_skills_data() -> dict:
- """Contains info on how skills should be updated"""
- skills_data_file = expanduser('~/.mycroft/skills.json')
- if isfile(skills_data_file):
- try:
- with open(skills_data_file) as f:
- return json.load(f)
- except json.JSONDecodeError:
- return {}
- else:
- return {}
-
-
-def write_skills_data(data: dict):
- skills_data_file = expanduser('~/.mycroft/skills.json')
- with open(skills_data_file, 'w') as f:
- json.dump(data, f, indent=4, separators=(',', ':'))
-
-
-def get_skill_entry(name, skills_data) -> dict:
- """ Find a skill entry in the skills_data and returns it. """
- for e in skills_data.get('skills', []):
- if e.get('name') == name:
- return e
- return {}
-
-
-def build_skill_entry(name, origin, beta, skill_gid) -> dict:
- """ Create a new skill entry
-
- Arguments:
- name: skill name
- origin: the source of the installation
- beta: Boolean indicating wether the skill is in beta
- skill_gid: skill global id
- Returns:
- populated skills entry
- """
- return {
- 'name': name,
- 'origin': origin,
- 'beta': beta,
- 'status': 'active',
- 'installed': 0,
- 'updated': 0,
- 'installation': 'installed',
- 'skill_gid': skill_gid
- }
-
-
-def skills_data_hash(data):
- return hash(json.dumps(data, sort_keys=True))
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn'
'--exclude=.svnignore' old/mycroft-skills-manager-0.7.8/msm/util.py
new/mycroft-skills-manager-0.8.2/msm/util.py
--- old/mycroft-skills-manager-0.7.8/msm/util.py 2019-07-09
16:28:02.000000000 +0200
+++ new/mycroft-skills-manager-0.8.2/msm/util.py 2019-08-23
12:31:42.000000000 +0200
@@ -19,6 +19,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+import time
+
import git
from os.path import exists
from os import chmod
@@ -46,3 +48,65 @@
lock_file.close()
chmod(lock_path, 0o777)
super().__init__(lock_path)
+
+# The cached_property class defined below was copied from the
+# PythonDecoratorLibrary at:
+# https://wiki.python.org/moin/PythonDecoratorLibrary/#Cached_Properties
+#
+# © 2011 Christopher Arndt, MIT License
+#
+class cached_property(object):
+ """Decorator for read-only properties evaluated only once within TTL
period.
+
+ It can be used to create a cached property like this::
+
+ import random
+
+ # the class containing the property must be a new-style class
+ class MyClass(object):
+ # create property whose value is cached for ten minutes
+ @cached_property(ttl=600)
+ def randint(self):
+ # will only be evaluated every 10 min. at maximum.
+ return random.randint(0, 100)
+
+ The value is cached in the '_cache' attribute of the object instance that
+ has the property getter method wrapped by this decorator. The '_cache'
+ attribute value is a dictionary which has a key for every property of the
+ object which is wrapped by this decorator. Each entry in the cache is
+ created only when the property is accessed for the first time and is a
+ two-element tuple with the last computed property value and the last time
+ it was updated in seconds since the epoch.
+
+ The default time-to-live (TTL) is 300 seconds (5 minutes). Set the TTL to
+ zero for the cached value to never expire.
+
+ To expire a cached property value manually just do::
+
+ del instance._cache[<property name>]
+
+ """
+ def __init__(self, ttl=300):
+ self.ttl = ttl
+
+ def __call__(self, fget, doc=None):
+ self.fget = fget
+ self.__doc__ = doc or fget.__doc__
+ self.__name__ = fget.__name__
+ self.__module__ = fget.__module__
+ return self
+
+ def __get__(self, inst, owner):
+ now = time.monotonic()
+ try:
+ value, last_update = inst._cache[self.__name__]
+ if self.ttl > 0 and now - last_update > self.ttl:
+ raise AttributeError
+ except (KeyError, AttributeError):
+ value = self.fget(inst)
+ try:
+ cache = inst._cache
+ except AttributeError:
+ cache = inst._cache = {}
+ cache[self.__name__] = (value, now)
+ return value
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn'
'--exclude=.svnignore' old/mycroft-skills-manager-0.7.8/setup.py
new/mycroft-skills-manager-0.8.2/setup.py
--- old/mycroft-skills-manager-0.7.8/setup.py 2019-07-09 16:28:02.000000000
+0200
+++ new/mycroft-skills-manager-0.8.2/setup.py 2019-08-23 12:31:42.000000000
+0200
@@ -23,7 +23,7 @@
setup(
name='msm',
- version='0.7.8',
+ version='0.8.2',
packages=['msm'],
install_requires=[
'GitPython', 'typing', 'fasteners', 'pyyaml', 'pako',
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn'
'--exclude=.svnignore' old/mycroft-skills-manager-0.7.8/tests/skills_test.json
new/mycroft-skills-manager-0.8.2/tests/skills_test.json
--- old/mycroft-skills-manager-0.7.8/tests/skills_test.json 1970-01-01
01:00:00.000000000 +0100
+++ new/mycroft-skills-manager-0.8.2/tests/skills_test.json 2019-08-23
12:31:42.000000000 +0200
@@ -0,0 +1,26 @@
+{
+ "blacklist":[],
+ "skills":[
+ {
+ "installed":12345,
+ "origin":"default",
+ "status":"active",
+ "name":"skill-foo",
+ "installation":"installed",
+ "skill_gid":"skill-foo|99.99",
+ "beta":false,
+ "updated":0
+ },
+ {
+ "installed":23456,
+ "origin":"default",
+ "status":"active",
+ "name":"skill-bar",
+ "installation":"installed",
+ "skill_gid":"skill-bar|99.99",
+ "beta":false,
+ "updated":0
+ }
+ ],
+ "version":2
+}
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn'
'--exclude=.svnignore'
old/mycroft-skills-manager-0.7.8/tests/test_mycroft_skills_manager.py
new/mycroft-skills-manager-0.8.2/tests/test_mycroft_skills_manager.py
--- old/mycroft-skills-manager-0.7.8/tests/test_mycroft_skills_manager.py
2019-07-09 16:28:02.000000000 +0200
+++ new/mycroft-skills-manager-0.8.2/tests/test_mycroft_skills_manager.py
2019-08-23 12:31:42.000000000 +0200
@@ -19,70 +19,409 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from shutil import rmtree
+import json
+import os
+import tempfile
+from pathlib import Path
+from shutil import copyfile, rmtree
+from unittest import TestCase
+from unittest.mock import call, Mock, patch
import pytest
-from os.path import join, dirname, exists, abspath
from msm import MycroftSkillsManager, AlreadyInstalled, AlreadyRemoved
-from msm.exceptions import SkillNotFound, MultipleSkillMatches
-from msm.skill_repo import SkillRepo
+from msm.exceptions import SkillNotFound, MultipleSkillMatches, MsmException
+from msm.skill_state import device_skill_state_hash
-class TestMycroftSkillsManager(object):
- def setup(self):
- self.root = root = dirname(abspath(__file__))
+class TestMycroftSkillsManager(TestCase):
+ def setUp(self):
+ temp_dir = tempfile.mkdtemp()
+ self.temp_dir = Path(temp_dir)
+ self.skills_json_path = self.temp_dir.joinpath('skills.json')
+ self.skills_dir = self.temp_dir.joinpath('skills')
+ self._build_fake_skills()
+ self._mock_skills_json_path()
+ self._mock_skill_entry()
+ self._mock_skill_repo()
+ copyfile('skills_test.json', str(self.skills_json_path))
self.msm = MycroftSkillsManager(
- platform='default', skills_dir=join(root, 'test-skills'),
- repo=SkillRepo(
- join(root, 'repo-instance'), branch='test-repo',
- url='https://github.com/mycroftai/mycroft-skills-manager'
- ), versioned=True
+ platform='default',
+ skills_dir=str(self.temp_dir.joinpath('skills')),
+ repo=self.skill_repo_mock,
+ versioned=True
)
+ def _build_fake_skills(self):
+ foo_skill_dir = self.skills_dir.joinpath('skill-foo')
+ foo_skill_dir.mkdir(parents=True)
+ foo_skill_dir.joinpath('__init__.py').touch()
+ bar_skill_dir = self.skills_dir.joinpath('skill-bar')
+ bar_skill_dir.mkdir(parents=True)
+ bar_skill_dir.joinpath('__init__.py').touch()
+
+ def _mock_log(self):
+ log_patch = patch('msm.mycroft_skills_manager.LOG')
+ self.addCleanup(log_patch.stop)
+ self.log_mock = log_patch.start()
+
+ def _mock_skills_json_path(self):
+ expanduser_patch = patch('msm.skill_state.expanduser')
+ self.addCleanup(expanduser_patch.stop)
+ self.skills_json_path_mock = expanduser_patch.start()
+ self.skills_json_path_mock.return_value = str(
+ self.temp_dir.joinpath('skills.json')
+ )
+
+ def _mock_skill_entry(self):
+ skill_entry_patch = patch(
+ 'msm.mycroft_skills_manager.SkillEntry.install',
+ spec=True
+ )
+ self.addCleanup(skill_entry_patch.stop)
+ self.skill_entry_mock = skill_entry_patch.start()
+
+ def _mock_skill_repo(self):
+ skill_repo_patch = patch(
+ 'msm.mycroft_skills_manager.SkillRepo',
+ spec=True
+
+ )
+ self.addCleanup(skill_repo_patch.stop)
+ self.skill_repo_mock = skill_repo_patch.start()
+ self.skill_repo_mock.skills_meta_info = {
+ 'https://skill_foo_url': None
+ }
+
def teardown(self):
- if exists(self.msm.skills_dir):
- rmtree(self.msm.skills_dir)
- if exists(self.msm.repo.path):
- rmtree(self.msm.repo.path)
+ rmtree(str(self.temp_dir))
+
+ def test_device_skill_state(self):
+ """Contents of skills.json are loaded into memory"""
+ state = self.msm.device_skill_state
+ initial_state = [
+ dict(
+ name='skill-foo',
+ origin='default',
+ beta=False,
+ status='active',
+ installed=12345,
+ updated=0,
+ installation='installed',
+ skill_gid='@|skill-foo'
+ ),
+ dict(
+ name='skill-bar',
+ origin='default',
+ beta=False,
+ status='active',
+ installed=23456,
+ updated=0,
+ installation='installed',
+ skill_gid='@|skill-bar'
+ )
+ ]
+
+ self.assertListEqual(initial_state, state['skills'])
+ self.assertListEqual([], state['blacklist'])
+ self.assertEqual(2, state['version'])
+
+ new_hash = device_skill_state_hash(self.msm.device_skill_state)
+ self.assertEqual(new_hash, self.msm.device_skill_state_hash)
+
+ def test_build_device_skill_state(self):
+ """No skill.json file so build one."""
+ os.remove(str(self.skills_json_path))
+ self.msm._device_skill_state = None
+ self.msm._init_skills_data()
+ state = self.msm.device_skill_state
+
+ initial_state = [
+ dict(
+ name='skill-bar',
+ origin='non-msm',
+ beta=False,
+ status='active',
+ installed=0,
+ updated=0,
+ installation='installed',
+ skill_gid='@|skill-bar'
+ ),
+ dict(
+ name='skill-foo',
+ origin='non-msm',
+ beta=False,
+ status='active',
+ installed=0,
+ updated=0,
+ installation='installed',
+ skill_gid='@|skill-foo'
+ )
+ ]
+
+ self.assertTrue(self.skills_json_path.exists())
+ with open(self.skills_json_path) as skills_json:
+ device_skill_state = json.load(skills_json)
+ self.assertListEqual(initial_state, state['skills'])
+ self.assertListEqual(initial_state, device_skill_state['skills'])
+ self.assertListEqual([], state['blacklist'])
+ self.assertListEqual([], device_skill_state['blacklist'])
+ self.assertEqual(2, state['version'])
+ self.assertEqual(2, device_skill_state['version'])
+ new_hash = device_skill_state_hash(self.msm.device_skill_state)
+ self.assertEqual(new_hash, self.msm.device_skill_state_hash)
+
+ def test_remove_from_device_skill_state(self):
+ """Remove a file no longer installed from the device's skill state.
+
+ Delete skill-bar from the local skills. This should trigger it being
+ removed from the device skill state.
+ """
+
+ del(self.msm.local_skills['skill-bar'])
+ self.msm._device_skill_state = None
+ state = self.msm.device_skill_state
+
+ initial_state = [
+ dict(
+ name='skill-foo',
+ origin='default',
+ beta=False,
+ status='active',
+ installed=12345,
+ updated=0,
+ installation='installed',
+ skill_gid='@|skill-foo'
+ )
+ ]
+
+ self.assertListEqual(initial_state, state['skills'])
+ self.assertListEqual([], state['blacklist'])
+ self.assertEqual(2, state['version'])
+
+ def test_skill_list(self):
+ """The skill.list() method is called."""
+ all_skills = self.msm.list()
+
+ skill_names = [skill.name for skill in all_skills]
+ self.assertIn('skill-foo', skill_names)
+ self.assertIn('skill-bar', skill_names)
+ self.assertEqual(2, len(all_skills))
+ self.assertIsNone(self.msm._local_skills)
+ self.assertIsNone(self.msm._default_skills)
+ self.assertEqual(all_skills, self.msm._all_skills)
def test_install(self):
- """Install by url or name"""
- self.msm.install('skill-a')
- with pytest.raises(AlreadyInstalled):
- self.msm.install('skill-a')
- self.msm.install('skill-b')
+ """Install a skill
+
+ Test that the install method was called on the skill being installed
+ and that the new skill was added to the device's skill state.
+ """
+ skill_to_install = self.skill_entry_mock()
+ skill_to_install.name = 'skill-test'
+ skill_to_install.skill_gid = 'test-skill|99.99'
+ skill_to_install.is_beta = False
+ with patch('msm.mycroft_skills_manager.isinstance') as isinstance_mock:
+ isinstance_mock.return_value = True
+ with patch('msm.mycroft_skills_manager.time') as time_mock:
+ time_mock.time.return_value = 100
+ self.msm.install(skill_to_install, origin='voice')
+
+ with open(self.skills_json_path) as skills_json:
+ device_skill_state = json.load(skills_json)
+
+ skill_test_state = dict(
+ name='skill-test',
+ origin='voice',
+ beta=False,
+ status='active',
+ installed=100,
+ updated=0,
+ installation='installed',
+ skill_gid='test-skill|99.99'
+ )
+ self.assertIn(skill_test_state, device_skill_state['skills'])
+ self.assertListEqual(
+ [call.install(None)],
+ skill_to_install.method_calls
+ )
+
+ def test_already_installed(self):
+ """Attempt install of skill already on the device.
+
+ When this happens, an AlreadyInstalled exception is raised and the
+ device skill state is not modified.
+ """
+ skill_to_install = self.skill_entry_mock()
+ skill_to_install.name = 'skill-foo'
+ skill_to_install.skill_gid = 'skill-foo|99.99'
+ skill_to_install.is_beta = False
+ skill_to_install.install = Mock(side_effect=AlreadyInstalled())
+ pre_install_hash = device_skill_state_hash(
+ self.msm.device_skill_state
+ )
+ with patch('msm.mycroft_skills_manager.isinstance') as isinstance_mock:
+ isinstance_mock.return_value = True
+ with self.assertRaises(AlreadyInstalled):
+ self.msm.install(skill_to_install)
+
+ self.assertIsNotNone(self.msm._local_skills)
+ self.assertIn('all_skills', self.msm._cache)
+ post_install_hash = device_skill_state_hash(
+ self.msm.device_skill_state
+ )
+ self.assertEqual(pre_install_hash, post_install_hash)
+
+ def test_install_failure(self):
+ """Install attempt fails for whatever reason
+
+ When an install fails, the installation will raise a MsmException. The
+ skill install will be saved to the device skill state as failed and
+ the error that caused the exception will be included in the state.
+ """
+ skill_to_install = self.skill_entry_mock()
+ skill_to_install.name = 'skill-test'
+ skill_to_install.skill_gid = 'skill-test|99.99'
+ skill_to_install.is_beta = False
+ skill_to_install.install = Mock(side_effect=MsmException('RED ALERT!'))
+ with patch('msm.mycroft_skills_manager.isinstance') as isinstance_mock:
+ isinstance_mock.return_value = True
+ self.msm.install(skill_to_install, origin='cli')
+
+ with open(self.skills_json_path) as skills_json:
+ device_skill_state = json.load(skills_json)
+
+ skill_test_state = dict(
+ name='skill-test',
+ origin='cli',
+ beta=False,
+ status='error',
+ installed=0,
+ updated=0,
+ installation='failed',
+ skill_gid='skill-test|99.99',
+ failure_message='RED ALERT!'
+ )
+ self.assertIn(skill_test_state, self.msm.device_skill_state['skills'])
+ self.assertIn(skill_test_state, device_skill_state['skills'])
+ self.assertListEqual(
+ [call.install(None)],
+ skill_to_install.method_calls
+ )
def test_remove(self):
- """Remove by url or name"""
- with pytest.raises(AlreadyRemoved):
- self.msm.remove('skill-a')
- self.msm.install('skill-a')
- self.msm.remove('skill-a')
+ """Remove a skill
+
+ Test that the remove method was called on the skill being installed
+ and that the new skill was removed from the device's skill state.
+ """
+ skill_to_remove = self.skill_entry_mock()
+ skill_to_remove.name = 'skill-foo'
+ pre_install_hash = device_skill_state_hash(
+ self.msm.device_skill_state
+ )
+ with patch('msm.mycroft_skills_manager.isinstance') as isinstance_mock:
+ isinstance_mock.return_value = True
+ self.msm.remove(skill_to_remove)
+
+ with open(self.skills_json_path) as skills_json:
+ device_skill_state = json.load(skills_json)
+
+ skill_names = [skill['name'] for skill in device_skill_state['skills']]
+ self.assertNotIn('skill_foo', skill_names)
+ skill_names = [
+ skill['name'] for skill in self.msm.device_skill_state['skills']
+ ]
+ self.assertNotIn('skill_foo', skill_names)
+ self.assertListEqual([call.remove()], skill_to_remove.method_calls)
+ self.assertNotIn('all_skills', self.msm._cache)
+ self.assertIsNone(self.msm._local_skills)
+ post_install_hash = device_skill_state_hash(
+ self.msm.device_skill_state
+ )
+ self.assertNotEqual(pre_install_hash, post_install_hash)
+
+ def test_already_removed(self):
+ """Attempt removal of skill already removed from the device.
+
+ When this happens, an AlreadyRemoved exception is raised and the
+ device skill state is not modified.
+ """
+ skill_to_remove = self.skill_entry_mock()
+ skill_to_remove.name = 'skill-foo'
+ skill_to_remove.remove = Mock(side_effect=AlreadyRemoved())
+ pre_install_hash = device_skill_state_hash(
+ self.msm.device_skill_state
+ )
+ with patch('msm.mycroft_skills_manager.isinstance') as isinstance_mock:
+ isinstance_mock.return_value = True
+ self.msm.remove(skill_to_remove)
+
+ self.assertListEqual([call.remove()], skill_to_remove.method_calls)
+ self.assertIsNotNone(self.msm._local_skills)
+ self.assertIn('all_skills', self.msm._cache)
+ post_install_hash = device_skill_state_hash(
+ self.msm.device_skill_state
+ )
+ self.assertEqual(pre_install_hash, post_install_hash)
+
+ def test_remove_failure(self):
+ """Skill removal attempt fails for whatever reason
+
+ When n removal fails, a MsmException is raised. The removal will not
+ be saved to the device skill state.
+ """
+ skill_to_remove = self.skill_entry_mock()
+ skill_to_remove.name = 'skill-test'
+ skill_to_remove.remove = Mock(side_effect=MsmException('RED ALERT!'))
+ pre_install_hash = device_skill_state_hash(
+ self.msm.device_skill_state
+ )
+ with patch('msm.mycroft_skills_manager.isinstance') as isinstance_mock:
+ isinstance_mock.return_value = True
+ with self.assertRaises(MsmException):
+ self.msm.remove(skill_to_remove)
+
+ self.assertListEqual(
+ [call.remove()],
+ skill_to_remove.method_calls
+ )
+ self.assertIsNotNone(self.msm._local_skills)
+ self.assertIn('all_skills', self.msm._cache)
+ post_install_hash = device_skill_state_hash(
+ self.msm.device_skill_state
+ )
+ self.assertEqual(pre_install_hash, post_install_hash)
def test_update(self):
- """Update all downloaded skills"""
- self.msm.install('skill-a')
- self.msm.update()
-
- def test_install_defaults(self):
- """Installs the default skills, updates all others"""
- assert not self.msm.find_skill('skill-a').is_local
- self.msm.install_defaults()
- assert self.msm.find_skill('skill-a').is_local
- assert not self.msm.find_skill('skill-b').is_local
- self.msm.platform = 'platform-1'
- self.msm.install_defaults()
- assert self.msm.find_skill('skill-b').is_local
-
- def test_list(self):
- all_skills = {
- 'skill-a', 'skill-b', 'skill-cd', 'skill-ce'
- }
- assert {i.name for i in self.msm.list()} == all_skills
+ """Remove a skill
- def test_find_skill(self):
- with pytest.raises(MultipleSkillMatches):
- self.msm.find_skill('skill-c')
- with pytest.raises(SkillNotFound):
- self.msm.find_skill('jsafpcq')
+ Test that the remove method was called on the skill being installed
+ and that the new skill was removed from the device's skill state.
+ """
+ skill_to_update = self.skill_entry_mock()
+ skill_to_update.name = 'skill-foo'
+ skill_to_update.is_beta = False
+ pre_install_hash = device_skill_state_hash(
+ self.msm.device_skill_state
+ )
+ with patch('msm.mycroft_skills_manager.time') as time_mock:
+ time_mock.time.return_value = 100
+ self.msm.update(skill_to_update)
+
+ with open(self.skills_json_path) as skills_json:
+ device_skill_state = json.load(skills_json)
+
+ skill_names = [skill['name'] for skill in device_skill_state['skills']]
+ self.assertIn('skill-foo', skill_names)
+
+ for skill in self.msm.device_skill_state['skills']:
+ if skill['name'] == 'skill-foo':
+ self.assertEqual(100, skill['updated'])
+ self.assertListEqual([call.update()], skill_to_update.method_calls)
+ self.assertNotIn('all_skills', self.msm._cache)
+ self.assertIsNone(self.msm._local_skills)
+ post_install_hash = device_skill_state_hash(
+ self.msm.device_skill_state
+ )
+ self.assertNotEqual(pre_install_hash, post_install_hash)
