Refactor default skill installation (#188)

* Add `get_neon_skills_data` to preload skill json
Update skill_utils tests

* Extend skills timeout to troubleshoot test failure

* Refactor skill util tests
Fallback to non-cached requests if returned 400

* Refactor skill utils to use OSM/OSI instead of git directly

neon_core/util/skill_utils.py

@@ -23,29 +23,75 @@
 # NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 # SOFTWARE,  EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
-from os.path import expanduser
+import json
+import requests
+
+from os import listdir
+from tempfile import mkdtemp
+from shutil import rmtree
+from os.path import expanduser, join
+from ovos_skills_manager.skill_entry import SkillEntry
 from ovos_skills_manager.osm import OVOSSkillsManager
-from ovos_skills_manager.session import SESSION as requests, set_github_token, clear_github_token
+from ovos_skills_manager.session import SESSION, set_github_token, clear_github_token
+from ovos_skills_manager.github import normalize_github_url, get_branch_from_github_url, download_url_from_github_url
+from ovos_skill_installer import download_extract_zip
 from neon_utils.configuration_utils import get_neon_skills_config
 from neon_utils import LOG
 
 
+def get_neon_skills_data(skill_meta_repository: str = "https://github.com/neongeckocom/neon_skills",
+                         branch: str = "master",
+                         repo_metadata_path: str = "skill_metadata") -> dict:
+    """
+    Get skill data from configured neon_skills repository.
+    :param skill_meta_repository: URL of skills repository containing metadata
+    :param branch: branch of repository to checkout
+    :param repo_metadata_path: Path to repo directory containing json metadata files
+    """
+    skills_data = dict()
+    temp_download_dir = mkdtemp()
+    zip_url = download_url_from_github_url(skill_meta_repository, branch)
+    base_dir = join(temp_download_dir, "neon_skill_meta")
+    download_extract_zip(zip_url, temp_download_dir, "neon_skill_meta.zip", base_dir)
+
+    meta_dir = join(base_dir, repo_metadata_path)
+    for entry in listdir(meta_dir):
+        with open(join(meta_dir, entry)) as f:
+            skill_entry = json.load(f)
+        skills_data[normalize_github_url(skill_entry["url"])] = skill_entry
+    rmtree(temp_download_dir)
+    return skills_data
+
+
 def install_skills_from_list(skills_to_install: list, config: dict = None):
     """
     Installs the passed list of skill URLs
-    :param skills_to_install: list or skill URLs to install
+    :param skills_to_install: list of skill URLs to install
     :param config: optional dict configuration
     """
     config = config or get_neon_skills_config()
     skill_dir = expanduser(config["directory"])
     osm = OVOSSkillsManager()
+    skills_catalog = get_neon_skills_data()
     token_set = False
     if config.get("neon_token"):
         token_set = True
         set_github_token(config["neon_token"])
     for url in skills_to_install:
         try:
-            osm.install_skill_from_url(url, skill_dir)
+            normalized_url = normalize_github_url(url)
+            # Check if this skill is in the Neon list
+            if normalized_url in skills_catalog:
+                branch = get_branch_from_github_url(url)
+                # Set URL and branch to requested spec
+                skills_catalog[normalized_url]["url"] = normalized_url
+                skills_catalog[normalized_url]["branch"] = branch
+                entry = SkillEntry.from_json(skills_catalog.get(normalized_url), False)
+            else:
+                LOG.warning(f"Requested Skill not in Neon skill store ({url})")
+                entry = osm.skill_entry_from_url(url)
+        # try:
+            osm.install_skill(entry, skill_dir)
             LOG.info(f"Installed {url} to {skill_dir}")
         except Exception as e:
             LOG.error(e)
@@ -70,8 +116,12 @@ def install_skills_default(config: dict = None):
 def get_remote_entries(url):
     """ parse url and return a list of SkillEntry,
      expects 1 skill per line, can be a skill_id or url"""
-    r = requests.get(url)
-    if r.status_code == 200:
+    r = SESSION.get(url)
+    if not r.ok:
+        LOG.warning(f"Cached response returned: {r.status_code}")
+        SESSION.cache.delete_url(r.url)
+        r = requests.get(url)
+    if r.ok:
         return [s for s in r.text.split("\n") if s.strip()]
     else:
         LOG.error(f"{url} request failed with code: {r.status_code}")

test/test_run_neon.py

@@ -131,7 +131,7 @@ def test_audio_module(self):
     #     self.assertIsInstance(data["success"], bool)
 
     def test_skills_module(self):
-        response = self.bus.wait_for_response(Message('mycroft.skills.is_ready'))
+        response = self.bus.wait_for_response(Message('mycroft.skills.is_ready'), timeout=10)
         self.assertTrue(response.data['status'])
 
         response = self.bus.wait_for_response(Message("skillmanager.list"), "mycroft.skills.list")

test/test_skill_utils.py

@@ -42,7 +42,7 @@
 ]
 SKILL_DIR = os.path.join(os.path.dirname(__file__), "test_skills")
 SKILL_CONFIG = {
-    "default_skills": "https://raw.githubusercontent.com/NeonGeckoCom/neon-skills-submodules/dev/.utilities/"
+    "default_skills": "https://raw.githubusercontent.com/NeonGeckoCom/neon_skills/master/skill_lists/"
                       "DEFAULT-SKILLS-DEV",
     "neon_token": os.environ.get("GITHUB_TOKEN"),
     "directory": SKILL_DIR
@@ -83,7 +83,15 @@ def test_install_skills_from_list_with_auth(self):
     def test_install_skills_default(self):
         install_skills_default(SKILL_CONFIG)
         skill_dirs = [d for d in os.listdir(SKILL_DIR) if os.path.isdir(os.path.join(SKILL_DIR, d))]
-        self.assertEqual(len(skill_dirs), len(get_remote_entries(SKILL_CONFIG["default_skills"])), f"{skill_dirs}\n\n{get_remote_entries(SKILL_CONFIG['default_skills'])}")
+        self.assertEqual(len(skill_dirs), len(get_remote_entries(SKILL_CONFIG["default_skills"])),
+                         f"{skill_dirs}\n\n{get_remote_entries(SKILL_CONFIG['default_skills'])}")
+
+    def test_get_neon_skills_data(self):
+        neon_skills = get_neon_skills_data()
+        self.assertIsInstance(neon_skills, dict)
+        for skill in neon_skills:
+            self.assertIsInstance(neon_skills[skill], dict)
+            self.assertEqual(skill, normalize_github_url(neon_skills[skill]["url"]))
 
 
 if __name__ == '__main__':