feat/skill_launcher #65
Conversation
move some logic out of core, add helpers to launch skills standalone
move some logic out of core, add helpers to launch skills standalone
Codecov Report
@@ Coverage Diff @@
## dev #65 +/- ##
=====================================
Coverage ? 0.00%
=====================================
Files ? 32
Lines ? 2731
Branches ? 0
=====================================
Hits ? 0
Misses ? 2731
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| BaseSkill, MycroftSkill, OVOSSkill, OVOSFallbackSkill, | ||
| OVOSCommonPlaybackSkill, OVOSFallbackSkill, CommonQuerySkill, ActiveSkill, | ||
| FallbackSkill, UniversalSkill, UniversalFallback | ||
| ] |
There was a problem hiding this comment.
this has been introduced to fix the check below for finding skill class in a python file, it is used to ensure we dont return the base class, previously checked for MycroftSkill only
| def get_default_skills_directory(): | ||
| """ return default directory to scan for skills | ||
|
|
||
| data_dir is always XDG_DATA_DIR |
There was a problem hiding this comment.
note, this used to check mycroft.conf and was marked for deprecation in 0.0.3, removed now
| for name, obj in skill_module.__dict__.items(): | ||
| if isclass(obj): | ||
| if any(issubclass(obj, c) for c in SKILL_BASE_CLASSES) and \ | ||
| not any(obj is c for c in SKILL_BASE_CLASSES): |
There was a problem hiding this comment.
updated as mentioned in comment above
| # in __init__ we need to manually call _startup | ||
| self.instance = skill_creator(bus=self.bus, | ||
| skill_id=self.skill_id) | ||
| # skills will have bus and skill_id available as soon as they call super() |
There was a problem hiding this comment.
this try: except: block is the main change in this PR compared with core
| self._enable_settings_manager = enable_settings_manager | ||
| self._init_event = Event() | ||
| self.name = name or self.__class__.__name__ | ||
| self.resting_name = None | ||
| self.skill_id = '' # will be set by SkillLoader, guaranteed unique | ||
| self.skill_id = skill_id # will be set by SkillLoader, guaranteed unique |
There was a problem hiding this comment.
What if the skill_id is not unique?
There was a problem hiding this comment.
by definition it needs to be, if it isnt you are loading the same skill twice.
core ensures it wont load skills twice, the priority is:
- plugin skills, skill_id in setup.py
- all xdg skills directories, folder name is skill_id
- empty folder disables skill from stack above (ie, folder named after a plugin skill_id disables that plugin skill)
if you are loading skills externally then you may get skills loaded twice, there is no protection against that
There was a problem hiding this comment.
it is something we need to consider in core, will revisit this later, getting fallback skills working with the new entrypoints is higher priority :)
move some logic out of core, add helpers to launch skills standalone
if skills provide skill_id and bus in their kwargs / create_skill method they will initialize like regular python objects as soon as possible, this removes the issues around skill_id not being available before initialize
new cli entrypoints allow running skills fully standalone, eg, in docker
companion PR in core imports from here and deprecates
mycroft.skills.skill_loader.pyUSAGE: ovos-skill-launcher {skill_id} [path/to/my/skill_id]supports passing a full path to a local skill to help in development