From 527cdc6e2cd412e73234faae05b1eb29575d27c4 Mon Sep 17 00:00:00 2001 From: miro Date: Tue, 28 May 2024 07:16:18 +0100 Subject: [PATCH 1/8] fix/mycroft_backwards_compat relates to https://github.com/OpenVoiceOS/ovos-core/issues/420 --- ovos_workshop/skills/mycroft_skill.py | 48 +++++++++++++++++++++++++-- ovos_workshop/skills/ovos.py | 2 +- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/ovos_workshop/skills/mycroft_skill.py b/ovos_workshop/skills/mycroft_skill.py index d60caeb1..2b92f877 100644 --- a/ovos_workshop/skills/mycroft_skill.py +++ b/ovos_workshop/skills/mycroft_skill.py @@ -15,9 +15,11 @@ import shutil from os.path import join, exists +from json_database import JsonStorage from ovos_bus_client import MessageBusClient, Message from ovos_utils.log import LOG, log_deprecation, deprecated from ovos_workshop.decorators.compat import backwards_compat +from ovos_workshop.filesystem import FileSystemAccess from ovos_workshop.skills.ovos import OVOSSkill, is_classic_core, _OVOSSkillMetaclass @@ -169,6 +171,46 @@ def __init__(self, name: str = None, bus: MessageBusClient = None, self.settings_write_path = None self.settings_manager = None + @property + def file_system(self) -> FileSystemAccess: + """ + Get an object that provides managed access to a local Filesystem. + """ + if not self._file_system: + self._file_system = FileSystemAccess(join('skills', self.name)) + LOG.warning(f"with MycroftSkill self.file_system does not respect self.skill_id, path: {self._file_system.path}") + return self._file_system + + @property + def settings(self) -> dict: + """ + Get settings specific to this skill + """ + if self._settings is None: + self._settings = JsonStorage(self.settings_write_path, + disable_lock=True) + LOG.warning(f"with MycroftSkill self.settings may not respect self.skill_id, path: {self._settings.path}") + return self._settings + + def _init_settings(self): + """ + Set up skill settings. Defines settings in the specified file path, + handles any settings passed to skill init, and starts watching the + settings file for changes. + """ + self.log.debug(f"initializing skill settings for {self.skill_id}") + + if self._settings is None: + self._settings = JsonStorage(self.settings_write_path, + disable_lock=True) + + # starting on ovos-core 0.0.8 a bus event is emitted + # all settings.json files are monitored for changes in ovos-core + self.add_event("ovos.skills.settings_changed", self._handle_settings_changed) + + if self._monitor_own_settings: + self._start_filewatcher() + def __init_settings_manager_classic(self): super()._init_settings_manager() from mycroft.skills.settings import SettingsMetaUploader @@ -206,7 +248,7 @@ def _init_settings(self): # patched due to functional (internal) differences under mycroft-core def __on_end_classic(self, message: Message, handler_info: str, - skill_data: dict): + skill_data: dict, is_intent: bool = False): # mycroft-core style settings if self.settings != self._initial_settings: try: @@ -222,11 +264,11 @@ def __on_end_classic(self, message: Message, handler_info: str, @backwards_compat(classic_core=__on_end_classic) def _on_event_end(self, message: Message, handler_info: str, - skill_data: dict): + skill_data: dict, is_intent: bool = False): """ Store settings and indicate that the skill handler has completed """ - return super()._on_event_end(message, handler_info, skill_data) + return super()._on_event_end(message, handler_info, skill_data, is_intent) # refactored - backwards compat + log warnings @property diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index 11f43e04..12ca1402 100644 --- a/ovos_workshop/skills/ovos.py +++ b/ovos_workshop/skills/ovos.py @@ -476,7 +476,7 @@ def file_system(self) -> FileSystemAccess: f"to correct this add kwargs __init__(bus=None, skill_id='') " f"to skill class {self.__class__.__name__}") self.log.error(simple_trace(traceback.format_stack())) - raise Exception('Accessed MycroftSkill.file_system in __init__') + raise Exception('Accessed OVOSSkill.file_system in __init__') @file_system.setter def file_system(self, fs: FileSystemAccess): From 473d460eb35f89c3837941abf18a2b262fc48626 Mon Sep 17 00:00:00 2001 From: miro Date: Tue, 28 May 2024 07:21:13 +0100 Subject: [PATCH 2/8] use legacy skill id convention --- ovos_workshop/skills/mycroft_skill.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/ovos_workshop/skills/mycroft_skill.py b/ovos_workshop/skills/mycroft_skill.py index 2b92f877..cd484857 100644 --- a/ovos_workshop/skills/mycroft_skill.py +++ b/ovos_workshop/skills/mycroft_skill.py @@ -12,15 +12,17 @@ # See the License for the specific language governing permissions and # limitations under the License. +import inspect import shutil -from os.path import join, exists +from os.path import join, exists, dirname from json_database import JsonStorage from ovos_bus_client import MessageBusClient, Message from ovos_utils.log import LOG, log_deprecation, deprecated + from ovos_workshop.decorators.compat import backwards_compat from ovos_workshop.filesystem import FileSystemAccess -from ovos_workshop.skills.ovos import OVOSSkill, is_classic_core, _OVOSSkillMetaclass +from ovos_workshop.skills.ovos import OVOSSkill, _OVOSSkillMetaclass class _SkillMetaclass(_OVOSSkillMetaclass): @@ -95,18 +97,12 @@ def __call__(cls, *args, **kwargs): skill_id = a if not skill_id: - LOG.warning("skill initialized without bus!! this is legacy " - "behaviour and requires you to call skill.bind(bus)" - " or skill._startup(skill_id, bus)\n" - "bus will be required starting on ovos-core 0.1.0") - return super().__call__(*args, **kwargs) - + LOG.error("skill initialized without skill_id!! this is legacy " + "behaviour. skill_id will be required starting on ovos-core 0.1.0") # by convention skill_id is the folder name # usually repo.author - # TODO - uncomment once above is deprecated - # skill_id = dirname(inspect.getfile(cls)).split("/")[-1] - # LOG.warning(f"missing skill_id, assuming folder name " - # f"convention: {skill_id}") + skill_id = dirname(inspect.getfile(cls)).split("/")[-1] + LOG.warning(f"missing skill_id, assuming folder name convention: {skill_id}") try: # skill follows latest best practices, From 59bc460b2718167306eda82f168f7fab533a126a Mon Sep 17 00:00:00 2001 From: JarbasAI <33701864+JarbasAl@users.noreply.github.com> Date: Fri, 31 May 2024 19:12:41 +0100 Subject: [PATCH 3/8] Update mycroft_skill.py --- ovos_workshop/skills/mycroft_skill.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ovos_workshop/skills/mycroft_skill.py b/ovos_workshop/skills/mycroft_skill.py index cd484857..48a00164 100644 --- a/ovos_workshop/skills/mycroft_skill.py +++ b/ovos_workshop/skills/mycroft_skill.py @@ -22,7 +22,7 @@ from ovos_workshop.decorators.compat import backwards_compat from ovos_workshop.filesystem import FileSystemAccess -from ovos_workshop.skills.ovos import OVOSSkill, _OVOSSkillMetaclass +from ovos_workshop.skills.ovos import OVOSSkill, is_classic_core, _OVOSSkillMetaclass class _SkillMetaclass(_OVOSSkillMetaclass): From c91bff28aaab434dce3ecff6846a6cf5256f28fd Mon Sep 17 00:00:00 2001 From: JarbasAI <33701864+JarbasAl@users.noreply.github.com> Date: Sat, 1 Jun 2024 00:29:50 +0100 Subject: [PATCH 4/8] setters --- ovos_workshop/skills/mycroft_skill.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ovos_workshop/skills/mycroft_skill.py b/ovos_workshop/skills/mycroft_skill.py index 48a00164..e4387107 100644 --- a/ovos_workshop/skills/mycroft_skill.py +++ b/ovos_workshop/skills/mycroft_skill.py @@ -176,7 +176,12 @@ def file_system(self) -> FileSystemAccess: self._file_system = FileSystemAccess(join('skills', self.name)) LOG.warning(f"with MycroftSkill self.file_system does not respect self.skill_id, path: {self._file_system.path}") return self._file_system - + + @file_system.setter + def file_system(self, val): + LOG.warning("you are not supposed to override self.file_system, expect breakage!") + self._file_system = val + @property def settings(self) -> dict: """ @@ -187,7 +192,12 @@ def settings(self) -> dict: disable_lock=True) LOG.warning(f"with MycroftSkill self.settings may not respect self.skill_id, path: {self._settings.path}") return self._settings - + + @settings.setter + def settings(self, val): + LOG.warning("you are not supposed to override self.settings, expect breakage!") + self._settings = val + def _init_settings(self): """ Set up skill settings. Defines settings in the specified file path, From 2d3871c5657d20a58d915c7c8a00236cc9276856 Mon Sep 17 00:00:00 2001 From: JarbasAI <33701864+JarbasAl@users.noreply.github.com> Date: Sat, 1 Jun 2024 21:45:41 +0100 Subject: [PATCH 5/8] logs --- ovos_workshop/skills/ovos.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index 12ca1402..d2ea4a2e 100644 --- a/ovos_workshop/skills/ovos.py +++ b/ovos_workshop/skills/ovos.py @@ -428,7 +428,8 @@ def settings(self) -> JsonStorage: 'can be set, no settings can be read or changed.' f"to correct this add kwargs " f"__init__(bus=None, skill_id='') " - f"to skill class {self.__class__.__name__}") + f"to skill class {self.__class__.__name__} " + "You can only use self.settings after the call to 'super()'") self.log.error(simple_trace(traceback.format_stack())) return self._initial_settings @@ -458,9 +459,10 @@ def enclosure(self) -> EnclosureAPI: self.log.warning('Skill not fully initialized.' f"to correct this add kwargs " f"__init__(bus=None, skill_id='') " - f"to skill class {self.__class__.__name__}") + f"to skill class {self.__class__.__name__}." + "You can only use self.enclosure after the call to 'super()'") self.log.error(simple_trace(traceback.format_stack())) - raise Exception('Accessed MycroftSkill.enclosure in __init__') + raise Exception('Accessed OVOSSkill.enclosure in __init__') @property def file_system(self) -> FileSystemAccess: @@ -474,7 +476,8 @@ def file_system(self) -> FileSystemAccess: else: self.log.warning('Skill not fully initialized.' f"to correct this add kwargs __init__(bus=None, skill_id='') " - f"to skill class {self.__class__.__name__}") + f"to skill class {self.__class__.__name__} " + "You can only use self.file_system after the call to 'super()'") self.log.error(simple_trace(traceback.format_stack())) raise Exception('Accessed OVOSSkill.file_system in __init__') @@ -501,9 +504,10 @@ def bus(self) -> MessageBusClient: self.log.warning('Skill not fully initialized.' f"to correct this add kwargs " f"__init__(bus=None, skill_id='') " - f"to skill class {self.__class__.__name__}") + f"to skill class {self.__class__.__name__} " + "You can only use self.bus after the call to 'super()'") self.log.error(simple_trace(traceback.format_stack())) - raise Exception('Accessed MycroftSkill.bus in __init__') + raise Exception('Accessed OVOSSkill.bus in __init__') @bus.setter def bus(self, value: MessageBusClient): From 2c4c38ea820dd805c607221ca3ae570d5f613897 Mon Sep 17 00:00:00 2001 From: JarbasAI <33701864+JarbasAl@users.noreply.github.com> Date: Sat, 1 Jun 2024 21:49:23 +0100 Subject: [PATCH 6/8] Update ovos.py --- ovos_workshop/skills/ovos.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index d2ea4a2e..a87b248c 100644 --- a/ovos_workshop/skills/ovos.py +++ b/ovos_workshop/skills/ovos.py @@ -438,6 +438,7 @@ def settings(self, val: dict): """ Update settings specific to this skill """ + LOG.warning("Skills are not supposed to override self.settings, expect breakage! Set individual dict keys instead") assert isinstance(val, dict) # init method if self._settings is None: @@ -489,8 +490,7 @@ def file_system(self, fs: FileSystemAccess): system directory. @param fs: new FileSystemAccess object to use """ - self.log.warning(f"Skill manually overriding file_system path to: " - f"{fs.path}") + LOG.warning(f"Skill manually overriding file_system path to: {fs.path}") self._file_system = fs @property From 5c8e20cfa3fae7532b9f2779c503e31b788fc85c Mon Sep 17 00:00:00 2001 From: JarbasAI <33701864+JarbasAl@users.noreply.github.com> Date: Sat, 1 Jun 2024 21:50:06 +0100 Subject: [PATCH 7/8] log consistency --- ovos_workshop/skills/mycroft_skill.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ovos_workshop/skills/mycroft_skill.py b/ovos_workshop/skills/mycroft_skill.py index e4387107..17cdb133 100644 --- a/ovos_workshop/skills/mycroft_skill.py +++ b/ovos_workshop/skills/mycroft_skill.py @@ -195,7 +195,7 @@ def settings(self) -> dict: @settings.setter def settings(self, val): - LOG.warning("you are not supposed to override self.settings, expect breakage!") + LOG.warning("Skills are not supposed to override self.settings, expect breakage! Set individual dict keys instead") self._settings = val def _init_settings(self): From 8f396bf59eea6fa5ec557d3d495723b2d7e226b2 Mon Sep 17 00:00:00 2001 From: JarbasAI <33701864+JarbasAl@users.noreply.github.com> Date: Sat, 1 Jun 2024 21:57:47 +0100 Subject: [PATCH 8/8] Update test_skill.py --- test/unittests/test_skill.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unittests/test_skill.py b/test/unittests/test_skill.py index 5087fffa..c4a9f692 100644 --- a/test/unittests/test_skill.py +++ b/test/unittests/test_skill.py @@ -178,7 +178,7 @@ def test_legacy(self): # a legacy skill not accepting args at all with self.assertRaises(Exception) as ctxt: BadLegacySkill() # accesses self.bus in __init__ - self.assertTrue("Accessed MycroftSkill.bus in __init__" in str(ctxt.exception)) + self.assertTrue("Accessed OVOSSkill.bus in __init__" in str(ctxt.exception)) legacynoargs = LegacySkill() # no exception this time because bus is not used in init self.assertTrue(legacynoargs.inited)