From a5e622759073c71f4f32f757ffb8b0a69446f162 Mon Sep 17 00:00:00 2001 From: miro Date: Mon, 18 Nov 2024 19:00:05 +0000 Subject: [PATCH 1/3] deprecate!:backend client --- ovos_workshop/settings.py | 162 ----------------------------- ovos_workshop/skills/ovos.py | 32 ------ test/unittests/skills/test_base.py | 3 - test/unittests/skills/test_ovos.py | 14 --- test/unittests/test_settings.py | 7 -- 5 files changed, 218 deletions(-) delete mode 100644 test/unittests/test_settings.py diff --git a/ovos_workshop/settings.py b/ovos_workshop/settings.py index 2a6c15c7..c33d8959 100644 --- a/ovos_workshop/settings.py +++ b/ovos_workshop/settings.py @@ -1,167 +1,5 @@ -import json -from os.path import isfile -from threading import Timer -from typing import Optional - -import yaml from json_database import JsonStorageXDG -from ovos_backend_client.api import DeviceApi -from ovos_backend_client.pairing import is_paired, requires_backend -from ovos_backend_client.settings import RemoteSkillSettings, get_display_name -from ovos_bus_client import MessageBusClient -from ovos_bus_client.message import Message, dig_for_message -from ovos_utils.log import LOG - - -class SkillSettingsManager: - def __init__(self, skill): - self.download_timer: Optional[Timer] = None - self.skill = skill - self.api = DeviceApi() - self.remote_settings = \ - RemoteSkillSettings(self.skill_id, - settings=dict(self.skill.settings), - meta=self.load_meta(), remote_id=self.skill_gid) - self.register_bus_handlers() - - def start(self): - self._download() - - def _download(self): - # If this method is called outside of the timer loop, ensure the - # existing timer is canceled before starting a new one. - if self.download_timer: - self.download_timer.cancel() - - self.download() - - # prepare to download again in 60 seconds - self.download_timer = Timer(60, self._download) - self.download_timer.daemon = True - self.download_timer.start() - - def stop(self): - # If this method is called outside of the timer loop, ensure the - # existing timer is canceled - if self.download_timer: - self.download_timer.cancel() - - @property - def bus(self) -> MessageBusClient: - return self.skill.bus - - @property - def skill_id(self) -> str: - return self.skill.skill_id - - @property - def display_name(self) -> str: - return get_display_name(self.skill_id) - - @property - def skill_gid(self) -> str: - return f"@{self.api.uuid}|{self.skill_id}" - - @property - def skill_meta(self) -> dict: - return self.remote_settings.meta - - def register_bus_handlers(self): - self.skill.add_event('mycroft.skills.settings.update', - self.handle_download_remote) # backwards compat - self.skill.add_event('mycroft.skills.settings.download', - self.handle_download_remote) - self.skill.add_event('mycroft.skills.settings.upload', - self.handle_upload_local) - self.skill.add_event('mycroft.skills.settings.upload.meta', - self.handle_upload_meta) - self.skill.add_event('mycroft.paired', - self.handle_upload_local) - - def load_meta(self) -> dict: - json_path = f"{self.skill.root_dir}/settingsmeta.json" - yaml_path = f"{self.skill.root_dir}/settingsmeta.yaml" - if isfile(yaml_path): - with open(yaml_path) as meta_file: - return yaml.safe_load(meta_file) - elif isfile(json_path): - with open(json_path) as meta_file: - return json.load(meta_file) - return {} - - def save_meta(self, generate: bool = False): - # unset reload flag to avoid a reload on settingmeta change - # TODO - support for settingsmeta XDG paths - reload = self.skill.reload_skill - self.skill.reload_skill = False - - # generate meta for missing fields - if generate: - self.remote_settings.generate_meta() - - # write to disk - json_path = f"{self.skill.root_dir}/settingsmeta.json" - yaml_path = f"{self.skill.root_dir}/settingsmeta.yaml" - if isfile(yaml_path): - with open(yaml_path) as meta_file: - yaml.dump(self.remote_settings.meta, meta_file) - else: - with open(json_path, "w") as meta_file: - json.dump(self.remote_settings.meta, meta_file) - - # reset reloading flag - self.skill.reload_skill = reload - - @requires_backend - def upload(self, generate: bool = False): - if not is_paired(): - LOG.debug("Device needs to be paired to upload settings") - return - self.remote_settings.settings = dict(self.skill.settings) - if generate: - self.remote_settings.generate_meta() - self.remote_settings.upload() - - @requires_backend - def upload_meta(self, generate: bool = False): - if not is_paired(): - LOG.debug("Device needs to be paired to upload settingsmeta") - return - if generate: - self.remote_settings.settings = dict(self.skill.settings) - self.remote_settings.generate_meta() - self.remote_settings.upload_meta() - - @requires_backend - def download(self): - if not is_paired(): - LOG.debug("Device needs to be paired to download remote settings") - return - self.remote_settings.download() - # we do not update skill object settings directly - # skill will handle the event and trigger a callback - if self.skill.settings != self.remote_settings.settings: - # dig old message to keep context - msg = dig_for_message() or Message("") - msg = msg.forward('mycroft.skills.settings.changed') - - msg.data[self.skill_id] = self.remote_settings.settings - self.bus.emit(msg) - - def handle_upload_meta(self, message: Message): - skill_id = message.data.get("skill_id") - if skill_id == self.skill_id: - self.upload_meta() - - def handle_upload_local(self, message: Message): - skill_id = message.data.get("skill_id") - if skill_id == self.skill_id: - self.upload() - - def handle_download_remote(self, message: Message): - self.download() - def settings2meta(settings, section_name="Skill Settings"): """ generates basic settingsmeta """ diff --git a/ovos_workshop/skills/ovos.py b/ovos_workshop/skills/ovos.py index 9d39f64d..ffcd7704 100644 --- a/ovos_workshop/skills/ovos.py +++ b/ovos_workshop/skills/ovos.py @@ -54,7 +54,6 @@ from ovos_workshop.resource_files import ResourceFile, \ CoreResources, find_resource, SkillResources from ovos_workshop.settings import PrivateSettings -from ovos_workshop.settings import SkillSettingsManager def simple_trace(stack_trace: List[str]) -> str: @@ -92,7 +91,6 @@ def __init__(self, name: Optional[str] = None, resources_dir: Optional[str] = None, settings: Optional[JsonStorage] = None, gui: Optional[GUIInterface] = None, - enable_settings_manager: bool = True, skill_id: str = ""): """ Create an OVOSSkill object. @@ -103,18 +101,12 @@ def __init__(self, name: Optional[str] = None, @param settings: Optional settings object, else defined in skill config path @param gui: Optional SkillGUI, else one is initialized - @param enable_settings_manager: if True, enables a SettingsManager for - this skill to manage default settings and backend sync @param skill_id: Unique ID for this skill """ - self.log = LOG # a dedicated namespace will be assigned in _startup - self._enable_settings_manager = enable_settings_manager self._init_event = Event() self.name = name or self.__class__.__name__ self.skill_id = skill_id # set by SkillLoader, guaranteed unique - self._settings_meta = None # DEPRECATED - backwards compat only - self.settings_manager = None self.private_settings = None # Get directory of skill source (__init__.py) @@ -811,8 +803,6 @@ def _startup(self, bus: MessageBusClient, skill_id: str = ""): self.status.set_alive() if not self.gui: self._init_skill_gui() - if self._enable_settings_manager: - self._init_settings_manager() self.load_data_files() self._register_skill_json() self._register_decorated() @@ -917,12 +907,6 @@ def _init_skill_gui(self): self.gui = SkillGUI(self) self.gui.setup_default_handlers() - def _init_settings_manager(self): - """ - Set up the SkillSettingsManager for this skill. - """ - self.settings_manager = SkillSettingsManager(self) - def register_homescreen_app(self, icon: str, name: str, event: str): """the icon file MUST be located under 'gui' subfolder""" # this path is hardcoded in ovos_gui.constants and follows XDG spec @@ -1025,19 +1009,6 @@ def _register_decorated(self): for intent_file in getattr(method, 'converse_intents'): self.register_converse_intent(intent_file, method) - def _upload_settings(self): - """ - Upload settings to a remote backend if configured. - """ - if self.settings_manager and self.config_core.get("skills", {}).get("sync2way"): - # upload new settings to backend - generate = self.config_core.get("skills", {}).get("autogen_meta", True) - # this will check global sync flag - self.settings_manager.upload(generate) - if generate: - # update settingsmeta file on disk - self.settings_manager.save_meta() - def bind(self, bus: MessageBusClient): """ Register MessageBusClient with skill. @@ -1154,7 +1125,6 @@ def _handle_settings_file_change(self, path: str): except Exception as e: self.log.exception("settings change callback failed, " f"file changes not handled!: {e}") - self._upload_settings() def handle_settings_change(self, message: Message): """ @@ -1334,8 +1304,6 @@ def default_shutdown(self): # Store settings if self.settings != self._initial_settings: self.settings.store() - if self._settings_meta: - self._settings_meta.stop() if self._settings_watchdog: self._settings_watchdog.shutdown() except Exception as e: diff --git a/test/unittests/skills/test_base.py b/test/unittests/skills/test_base.py index fe30c1a3..38ae5b5c 100644 --- a/test/unittests/skills/test_base.py +++ b/test/unittests/skills/test_base.py @@ -26,7 +26,6 @@ def tearDownClass(cls) -> None: shutil.rmtree(cls.test_config_path) def test_00_skill_init(self): - from ovos_workshop.settings import SkillSettingsManager from ovos_workshop.skills.ovos import SkillGUI from ovos_utils.events import EventContainer, EventSchedulerInterface from ovos_workshop.intents import IntentServiceInterface @@ -36,10 +35,8 @@ def test_00_skill_init(self): from ovos_workshop.resource_files import SkillResources self.assertIsInstance(self.skill.log, Logger) - self.assertIsInstance(self.skill._enable_settings_manager, bool) self.assertEqual(self.skill.name, self.skill.__class__.__name__) self.assertEqual(self.skill.skill_id, self.skill_id) - self.assertIsInstance(self.skill.settings_manager, SkillSettingsManager) self.assertTrue(isdir(self.skill.root_dir)) self.assertEqual(self.skill.res_dir, self.skill.root_dir) self.assertIsInstance(self.skill.gui, SkillGUI) diff --git a/test/unittests/skills/test_ovos.py b/test/unittests/skills/test_ovos.py index 1b2246e6..434e642a 100644 --- a/test/unittests/skills/test_ovos.py +++ b/test/unittests/skills/test_ovos.py @@ -6,7 +6,6 @@ from ovos_workshop.decorators.layers import IntentLayers from ovos_workshop.resource_files import SkillResources -from ovos_workshop.settings import SkillSettingsManager from ovos_workshop.skills.ovos import OVOSSkill @@ -98,19 +97,6 @@ def test_send_stop_signal(self): # TODO pass - def test_settings_manager_init(self): - bus = FakeBus() - skill_default = MockSkill(bus=bus) - skill_default._startup(bus) - - self.assertIsInstance(skill_default.settings_manager, - SkillSettingsManager) - - skill_disabled_settings = MockSkill(bus=bus, - enable_settings_manager=False) - skill_disabled_settings._startup(bus) - self.assertIsNone(skill_disabled_settings.settings_manager) - def test_bus_setter(self): bus = FakeBus() skill = MockSkill() diff --git a/test/unittests/test_settings.py b/test/unittests/test_settings.py deleted file mode 100644 index e27173bb..00000000 --- a/test/unittests/test_settings.py +++ /dev/null @@ -1,7 +0,0 @@ -import unittest - - -class TestSettings(unittest.TestCase): - from ovos_workshop.settings import SkillSettingsManager - # TODO - From bdaee47329cb5b05074211eccc821039963bdf49 Mon Sep 17 00:00:00 2001 From: miro Date: Mon, 18 Nov 2024 19:10:59 +0000 Subject: [PATCH 2/3] deprecate!:backend client --- requirements/requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements/requirements.txt b/requirements/requirements.txt index 67e20e38..772306a8 100644 --- a/requirements/requirements.txt +++ b/requirements/requirements.txt @@ -1,7 +1,6 @@ ovos-utils>= 0.2.1,<1.0.0 ovos_bus_client>=0.0.8,<2.0.0 ovos-config>=0.0.12,<1.0.0 -ovos-backend-client>=0.1.0,<2.0.0 ovos-solver-yes-no-plugin>=0.0.1,<1.0.0 ovos-number-parser>=0.0.1,<1.0.0 rapidfuzz From 193df9e5f539a8866503e35c637a5cb95c32f55f Mon Sep 17 00:00:00 2001 From: miro Date: Mon, 18 Nov 2024 19:26:15 +0000 Subject: [PATCH 3/3] deprecate!:backend client --- ovos_workshop/app.py | 16 +--------------- test/unittests/skills/test_base.py | 10 ---------- test/unittests/test_abstract_app.py | 26 +------------------------- 3 files changed, 2 insertions(+), 50 deletions(-) diff --git a/ovos_workshop/app.py b/ovos_workshop/app.py index 98e2acf5..6f0c8119 100644 --- a/ovos_workshop/app.py +++ b/ovos_workshop/app.py @@ -13,9 +13,7 @@ class OVOSAbstractApplication(OVOSSkill): def __init__(self, skill_id: str, bus: Optional[MessageBusClient] = None, resources_dir: Optional[str] = None, - lang=None, settings: Optional[dict] = None, - gui: Optional[GUIInterface] = None, - enable_settings_manager: bool = False, **kwargs): + gui: Optional[GUIInterface] = None, **kwargs): """ Create an Application. An application is essentially a skill, but designed such that it may be run without an intent service. @@ -23,11 +21,7 @@ def __init__(self, skill_id: str, bus: Optional[MessageBusClient] = None, @param bus: MessageBusClient to bind to application @param resources_dir: optional root resource directory (else defaults to application `root_dir` - @param lang: DEPRECATED language of the application - @param settings: DEPRECATED settings object @param gui: GUIInterface to bind (if `None`, one is created) - @param enable_settings_manager: if True, enables a SettingsManager for - this application to manage default settings and backend sync """ self._dedicated_bus = False if bus: @@ -38,15 +32,7 @@ def __init__(self, skill_id: str, bus: Optional[MessageBusClient] = None, super().__init__(skill_id=skill_id, bus=bus, gui=gui, resources_dir=resources_dir, - enable_settings_manager=enable_settings_manager, **kwargs) - - if settings: - log_deprecation(f"Settings should be set in {self.settings_path}. " - f"Passing `settings` to __init__ is not supported.", - "0.1.0") - self.settings.merge(settings) - @property def settings_path(self) -> str: """ diff --git a/test/unittests/skills/test_base.py b/test/unittests/skills/test_base.py index 38ae5b5c..de9a960b 100644 --- a/test/unittests/skills/test_base.py +++ b/test/unittests/skills/test_base.py @@ -167,28 +167,18 @@ def test_upload_settings(self): pass def test_handle_settings_file_change(self): - real_upload = self.skill._upload_settings - self.skill._upload_settings = Mock() settings_file = self.skill.settings.path - # Handle change with no callback - self.skill._handle_settings_file_change(settings_file) - self.skill._upload_settings.assert_called_once() - # Handle change with callback - self.skill._upload_settings.reset_mock() self.skill.settings_change_callback = Mock() self.skill._handle_settings_file_change(settings_file) - self.skill._upload_settings.assert_called_once() self.skill.settings_change_callback.assert_called_once() # Handle non-settings file change self.skill._handle_settings_file_change(join(dirname(settings_file), "test.file")) - self.skill._upload_settings.assert_called_once() self.skill.settings_change_callback.assert_called_once() - self.skill._upload_settings = real_upload def test_load_lang(self): # TODO diff --git a/test/unittests/test_abstract_app.py b/test/unittests/test_abstract_app.py index 216f510f..965f31eb 100644 --- a/test/unittests/test_abstract_app.py +++ b/test/unittests/test_abstract_app.py @@ -19,33 +19,9 @@ def __int__(self, *args, **kwargs): class TestApp(unittest.TestCase): bus = FakeBus() - test_path = join(dirname(__file__), "test_config.json") - settings = {'test': True, - 'updated': False} - settings_obj = JsonStorage(test_path, True) - settings_obj.update(settings) - gui = GUIInterface("TestApplication") - app = Application(skill_id="TestApplication", settings=settings_obj, - gui=gui, bus=bus) - - def test_settings_manager_init(self): - self.assertIsNone(self.app.settings_manager) - - def test_settings_init(self): - self.assertNotEqual(self.app.settings, self.settings_obj) - self.assertFalse(self.app.settings['__mycroft_skill_firstrun']) - self.assertTrue(self.app.settings['test']) - self.assertFalse(self.app.settings['updated']) - self.settings_obj['updated'] = True - self.assertFalse(self.app.settings['updated']) - - def test_settings_init_invalid_arg(self): - app = Application(skill_id="TestApplication", bus=self.bus, - settings=self.settings) - self.assertNotEqual(app.settings, self.settings) - self.assertFalse(app.settings['__mycroft_skill_firstrun']) + app = Application(skill_id="TestApplication", gui=gui, bus=bus) def test_gui_init(self): self.assertEqual(self.app.gui, self.gui)