From 0471ade756d6ccbd1d99e9dc163da98c5f741030 Mon Sep 17 00:00:00 2001 From: maaktweluit <10008353+maaktweluit@users.noreply.github.com> Date: Fri, 21 Feb 2020 13:29:37 +0100 Subject: [PATCH 1/9] Added missing teardowns in test that use database and dir fixtures --- tests/golem/core/test_fileshelper.py | 3 +++ tests/golem/network/concent/test_received_handler.py | 2 ++ tests/golem/verifier/test_blenderverifier.py | 2 ++ 3 files changed, 7 insertions(+) diff --git a/tests/golem/core/test_fileshelper.py b/tests/golem/core/test_fileshelper.py index 4064585bf4..b8a1b0b737 100644 --- a/tests/golem/core/test_fileshelper.py +++ b/tests/golem/core/test_fileshelper.py @@ -210,6 +210,8 @@ def tearDown(self): if os.path.isdir(self.testdir): shutil.rmtree(self.testdir) + super().tearDown() + class TestDu(TestDirFixture): @@ -290,6 +292,7 @@ def tearDown(self): os.rmdir(self.test_dir2) os.rmdir(self.test_dir1) + super().tearDown() def test_find_file_with_ext(self): """ Test find_file_with_ext method """ diff --git a/tests/golem/network/concent/test_received_handler.py b/tests/golem/network/concent/test_received_handler.py index 039b927c5a..d5f559a880 100644 --- a/tests/golem/network/concent/test_received_handler.py +++ b/tests/golem/network/concent/test_received_handler.py @@ -187,6 +187,8 @@ def tearDown(self): # Remove registered handlers del self.task_server gc.collect() + testutils.DatabaseFixture.tearDown(self) + testutils.TestWithClient.tearDown(self) class IsOursTest(TaskServerMessageHandlerTestBase): diff --git a/tests/golem/verifier/test_blenderverifier.py b/tests/golem/verifier/test_blenderverifier.py index 44e65b1a4e..1b8bcfd33d 100644 --- a/tests/golem/verifier/test_blenderverifier.py +++ b/tests/golem/verifier/test_blenderverifier.py @@ -29,6 +29,7 @@ logger = logging.getLogger(__name__) + @pytest.mark.slow @pytest.mark.skipif( not is_linux(), @@ -71,6 +72,7 @@ def tearDown(self): # Try again after 3 seconds sleep(3) self.remove_files() + super().tearDown() def remove_files(self): above_tmp_dir = os.path.dirname(self.tempdir) From f7ed4904b90d69b8c3a83723a62c3c2363f72ad6 Mon Sep 17 00:00:00 2001 From: maaktweluit <10008353+maaktweluit@users.noreply.github.com> Date: Fri, 21 Feb 2020 13:30:46 +0100 Subject: [PATCH 2/9] Added rpc calls for managing apps --- golem/apps/__init__.py | 17 ++++++ golem/apps/default.py | 16 +++--- golem/apps/manager.py | 33 ++++++++++- golem/apps/rpc.py | 54 +++++++++++++++++ golem/client.py | 3 + golem/task/taskserver.py | 13 +---- tests/golem/apps/test_app_manager.py | 12 +++- tests/golem/apps/test_app_rpc.py | 86 ++++++++++++++++++++++++++++ 8 files changed, 211 insertions(+), 23 deletions(-) create mode 100644 golem/apps/rpc.py create mode 100644 tests/golem/apps/test_app_rpc.py diff --git a/golem/apps/__init__.py b/golem/apps/__init__.py index 5097c6464c..40c0f629f7 100644 --- a/golem/apps/__init__.py +++ b/golem/apps/__init__.py @@ -6,6 +6,7 @@ from dataclasses import dataclass, field from dataclasses_json import dataclass_json, config from marshmallow import fields as mm_fields +from pathvalidate import sanitize_filename from golem.marketplace import ( RequestorMarketStrategy, @@ -89,3 +90,19 @@ def load_apps_from_dir(app_dir: Path) -> Iterator[AppDefinition]: yield load_app_from_json_file(json_file) except ValueError: continue + + +def delete_app_from_dir(app_dir, app_def: AppDefinition) -> bool: + filename = app_json_file_name(app_def) + file = app_dir / filename + if not file.exists(): + logger.warning('Can not delete app, file not found. file=%r', file) + return False + file.unlink() + return True + + +def app_json_file_name(app_def: AppDefinition) -> str: + filename = f"{app_def.name}_{app_def.version}_{app_def.id}.json" + filename = sanitize_filename(filename, replacement_text="_") + return filename diff --git a/golem/apps/default.py b/golem/apps/default.py index 9c5f90e25a..5a8874b81f 100644 --- a/golem/apps/default.py +++ b/golem/apps/default.py @@ -2,9 +2,10 @@ from typing import List from golem_task_api.envs import DOCKER_CPU_ENV_ID -from pathvalidate import sanitize_filename -from golem.apps import AppId, AppDefinition, save_app_to_json_file +from golem.apps import ( + AppId, AppDefinition, save_app_to_json_file, app_json_file_name, +) from golem.marketplace import RequestorBrassMarketStrategy BlenderAppDefinition = AppDefinition( @@ -31,16 +32,13 @@ def save_built_in_app_definitions(path: Path) -> List[AppId]: - app_ids = [] - + new_app_ids = [] for app_id, app in APPS.items(): - - filename = f"{app.name}_{app.version}_{app_id}.json" - filename = sanitize_filename(filename, replacement_text="_") + filename = app_json_file_name(app) json_file = path / filename if not json_file.exists(): save_app_to_json_file(app, json_file) - app_ids.append(app_id) + new_app_ids.append(app_id) - return app_ids + return new_app_ids diff --git a/golem/apps/manager.py b/golem/apps/manager.py index 3b5bb1129e..322575295e 100644 --- a/golem/apps/manager.py +++ b/golem/apps/manager.py @@ -1,7 +1,11 @@ import logging from typing import Dict, List, Tuple +from pathlib import Path -from golem.apps import AppId, AppDefinition +from golem.apps import ( + AppId, AppDefinition, load_apps_from_dir, delete_app_from_dir, +) +from golem.apps.default import save_built_in_app_definitions from golem.model import AppConfiguration logger = logging.getLogger(__name__) @@ -10,9 +14,22 @@ class AppManager: """ Manager class for applications using Task API. """ - def __init__(self) -> None: + def __init__(self, app_dir: Path, save_apps=True) -> None: self._apps: Dict[AppId, AppDefinition] = {} self._state = AppStates() + self._app_dir = app_dir + + # Save build in apps, then load apps from path + built_in_apps = [] + if save_apps: + built_in_apps = save_built_in_app_definitions(app_dir) + for app_def in load_apps_from_dir(app_dir): + self.register_app(app_def) + for app_id in built_in_apps: + self.set_enabled(app_id, True) + + def registered(self, app_id) -> bool: + return app_id in self._apps def register_app(self, app: AppDefinition) -> None: """ Register an application in the manager. """ @@ -58,6 +75,18 @@ def app(self, app_id: AppId) -> AppDefinition: """ Get an app with given ID (assuming it is registered). """ return self._apps[app_id] + def delete(self, app_id: AppId) -> bool: + # Delete self._state from the database first + try: + AppConfiguration.delete() \ + .where(AppConfiguration.app_id == app_id).execute() + except AppConfiguration.DoesNotExist: + logger.warning('Can not delete app, not found. id=%e', app_id) + return False + delete_app_from_dir(self._app_dir, self._apps[app_id]) + del self._apps[app_id] + return True + class AppStates: diff --git a/golem/apps/rpc.py b/golem/apps/rpc.py new file mode 100644 index 0000000000..8c74b5dfbc --- /dev/null +++ b/golem/apps/rpc.py @@ -0,0 +1,54 @@ +import logging +import typing + +from golem.rpc import utils as rpc_utils + +if typing.TYPE_CHECKING: + # pylint:disable=unused-import, ungrouped-imports + from golem.client import Client + +logger = logging.getLogger(__name__) + + +class ClientAppProvider: + def __init__(self, client: 'Client'): + self.app_manager = client.task_server.app_manager + + @rpc_utils.expose('apps.list') + def apps_list(self): + logger.debug('apps.list called from rpc') + result = [] + for app_id, app_def in self.app_manager.apps(): + logger.debug('app_id=%r, app_def=%r', app_id, app_def) + app_result = { + 'id': app_id, + 'name': app_def.name, + 'version': app_def.version, + 'enabled': self.app_manager.enabled(app_id), + } + # TODO: Add full argument for more values + result.append(app_result) + logger.info('Listing apps. count=%r', len(result)) + return result + + @rpc_utils.expose('apps.update') + def apps_update(self, app_id, enabled): + logger.debug( + 'apps.update called from rpc. app_id=%r, enabled=%r', + app_id, + enabled, + ) + if not self.app_manager.registered(app_id): + logger.warning('App not found, not updated. app_id=%r', app_id) + return f"App not found, please check the app_id={app_id}" + self.app_manager.set_enabled(app_id, bool(enabled)) + logger.info('Updated app. app_id=%r, enabled=%r', app_id, enabled) + return "App state updated." + + @rpc_utils.expose('apps.delete') + def apps_delete(self, app_id): + logger.debug('apps.delete called from rpc. app_id=%r', app_id) + if not self.app_manager.delete(app_id): + return "Failed to delete app." + logger.info('Deleted app. app_id=%r', app_id) + return "App deleted with success." diff --git a/golem/client.py b/golem/client.py index ef76e70187..5df6319813 100644 --- a/golem/client.py +++ b/golem/client.py @@ -36,6 +36,7 @@ from golem import model from golem.appconfig import TASKARCHIVE_MAINTENANCE_INTERVAL, AppConfig from golem.apps.default import APPS +import golem.apps.rpc as apps_rpc from golem.clientconfigdescriptor import ConfigApprover, ClientConfigDescriptor from golem.core import variables from golem.core.common import ( @@ -266,6 +267,7 @@ def get_wamp_rpc_mapping(self): from golem.rpc.api import ethereum_ as api_ethereum from golem.task import rpc as task_rpc task_rpc_provider = task_rpc.ClientProvider(self) + app_rpc_provider = apps_rpc.ClientAppProvider(self) providers = ( self, concent_soft_switch, @@ -276,6 +278,7 @@ def get_wamp_rpc_mapping(self): self.environments_manager, self.transaction_system, task_rpc_provider, + app_rpc_provider, api_ethereum.ETSProvider(self.transaction_system), ) mapping = {} diff --git a/golem/task/taskserver.py b/golem/task/taskserver.py index a6c4a17468..5754b7a2db 100644 --- a/golem/task/taskserver.py +++ b/golem/task/taskserver.py @@ -32,12 +32,10 @@ from twisted.internet.defer import inlineCallbacks, Deferred, \ TimeoutError as DeferredTimeoutError -import golem.apps from apps.appsmanager import AppsManager from apps.core.task.coretask import CoreTask from golem import constants as gconst from golem.apps import manager as app_manager -from golem.apps.default import save_built_in_app_definitions from golem.clientconfigdescriptor import ClientConfigDescriptor from golem.core.common import ( short_node_id, @@ -154,14 +152,7 @@ def __init__( dev_mode=task_api_dev_mode, ) - app_dir = self.get_app_dir() - built_in_apps = save_built_in_app_definitions(app_dir) - - self.app_manager = app_mgr = app_manager.AppManager() - for app_def in golem.apps.load_apps_from_dir(app_dir): - app_mgr.register_app(app_def) - for app_id in built_in_apps: - app_mgr.set_enabled(app_id, True) + self.app_manager = app_manager.AppManager(self.get_app_dir()) self.node = node self.task_archiver = task_archiver @@ -182,7 +173,7 @@ def __init__( ) self.requested_task_manager = RequestedTaskManager( - app_manager=app_mgr, + app_manager=self.app_manager, env_manager=new_env_manager, public_key=self.keys_auth.public_key, root_path=Path(TaskServer.__get_task_manager_root(client.datadir)), diff --git a/tests/golem/apps/test_app_manager.py b/tests/golem/apps/test_app_manager.py index 07db03fc2b..7a34982d86 100644 --- a/tests/golem/apps/test_app_manager.py +++ b/tests/golem/apps/test_app_manager.py @@ -1,3 +1,5 @@ +from pathlib import Path + from golem.apps.manager import AppManager from golem.apps import ( AppDefinition, @@ -22,7 +24,9 @@ class AppManagerTestBase(DatabaseFixture): def setUp(self): super().setUp() - self.app_manager = AppManager() + app_path = self.new_path / 'apps' + app_path.mkdir(exist_ok=True) + self.app_manager = AppManager(app_path, False) class TestRegisterApp(AppManagerTestBase): @@ -38,6 +42,12 @@ def test_re_register(self): with self.assertRaises(ValueError): self.app_manager.register_app(APP_DEF) + def test_delete_app(self): + self.app_manager.register_app(APP_DEF) + self.assertEqual(self.app_manager.apps(), [(APP_ID, APP_DEF)]) + self.app_manager.delete(APP_ID) + self.assertEqual(self.app_manager.apps(), []) + class TestSetEnabled(AppManagerTestBase): diff --git a/tests/golem/apps/test_app_rpc.py b/tests/golem/apps/test_app_rpc.py new file mode 100644 index 0000000000..59f46bd837 --- /dev/null +++ b/tests/golem/apps/test_app_rpc.py @@ -0,0 +1,86 @@ +import pytest +from mock import Mock + +from golem.apps.rpc import ClientAppProvider +from golem.apps.manager import AppManager +from golem.apps.default import BlenderAppDefinition +from golem.client import Client +from golem.testutils import pytest_database_fixture # noqa pylint: disable=unused-import +from golem.task.taskserver import TaskServer + + +class TestClientAppProvider: + @pytest.fixture(autouse=True) + def setup_method(self): + self._client = Mock(spec=Client) + self._client.task_server = Mock(spec=TaskServer) + self._client.task_server.app_manager = Mock(spec=AppManager) + self._app_manger = self._client.task_server.app_manager + self._handler = ClientAppProvider(self._client) + + def test_list(self): + # given + mocked_apps = [(BlenderAppDefinition.id, BlenderAppDefinition)] + self._app_manger.apps = Mock( + return_value=mocked_apps + ) + + # when + result = self._handler.apps_list() + + # then + assert len(result) == len(mocked_apps), \ + 'count of result does not match input count' + assert result[0]['id'] == mocked_apps[0][0], \ + 'the first returned app id does not match input' + assert self._app_manger.apps.called_once_with() + + def test_update(self): + # given + app_id = 'a' + enabled = True + + # when + result = self._handler.apps_update(app_id, enabled) + + # then + self._app_manger.registered.called_once_with(app_id) + self._app_manger.set_enabled.called_once_with(app_id, enabled) + assert result == 'App state updated.' + + def test_update_not_registered(self): + # given + app_id = 'a' + enabled = True + self._app_manger.registered.return_value = False + + # when + result = self._handler.apps_update(app_id, enabled) + + # then + self._app_manger.registered.called_once_with(app_id) + self._app_manger.set_enabled.assert_not_called() + assert result == f"App not found, please check the app_id={app_id}" + + def test_delete(self): + # given + app_id = 'a' + + # when + result = self._handler.apps_delete(app_id) + + # then + self._app_manger.delete.called_once_with(app_id) + assert result == 'App deleted with success.' + + def test_delete_failed(self): + # given + app_id = 'a' + self._app_manger.delete.return_value = False + + # when + result = self._handler.apps_delete(app_id) + + # then + self._app_manger.delete.called_once_with(app_id) + assert result == 'Failed to delete app.' From 4cdbf114448885119a7214191ef027c1aa7ca447 Mon Sep 17 00:00:00 2001 From: maaktweluit <10008353+maaktweluit@users.noreply.github.com> Date: Fri, 21 Feb 2020 16:18:16 +0100 Subject: [PATCH 3/9] Review comments: super() like setUp, __delitem__ for AppState, consistent names in AppState parameters --- golem/apps/manager.py | 37 ++++++++++--------- .../network/concent/test_received_handler.py | 3 +- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/golem/apps/manager.py b/golem/apps/manager.py index 322575295e..2bfab08ae2 100644 --- a/golem/apps/manager.py +++ b/golem/apps/manager.py @@ -77,12 +77,7 @@ def app(self, app_id: AppId) -> AppDefinition: def delete(self, app_id: AppId) -> bool: # Delete self._state from the database first - try: - AppConfiguration.delete() \ - .where(AppConfiguration.app_id == app_id).execute() - except AppConfiguration.DoesNotExist: - logger.warning('Can not delete app, not found. id=%e', app_id) - return False + del self._state[app_id] delete_app_from_dir(self._app_dir, self._apps[app_id]) del self._apps[app_id] return True @@ -90,23 +85,23 @@ def delete(self, app_id: AppId) -> bool: class AppStates: - def __contains__(self, item): - if not isinstance(item, str): - self._raise_no_str_type(item) + def __contains__(self, key): + if not isinstance(key, str): + self._raise_no_str_type(key) return AppConfiguration.select(AppConfiguration.app_id) \ - .where(AppConfiguration.app_id == item) \ + .where(AppConfiguration.app_id == key) \ .exists() - def __getitem__(self, item): - if not isinstance(item, str): - self._raise_no_str_type(item) + def __getitem__(self, key): + if not isinstance(key, str): + self._raise_no_str_type(key) try: return AppConfiguration \ - .get(AppConfiguration.app_id == item) \ + .get(AppConfiguration.app_id == key) \ .enabled except AppConfiguration.DoesNotExist: - raise KeyError(item) + raise KeyError(key) def __setitem__(self, key, val): if not isinstance(key, str): @@ -116,6 +111,14 @@ def __setitem__(self, key, val): AppConfiguration.insert(app_id=key, enabled=val).upsert().execute() + def __delitem__(self, key): + try: + AppConfiguration.delete() \ + .where(AppConfiguration.app_id == key).execute() + except AppConfiguration.DoesNotExist: + logger.warning('Can not delete app, not found. id=%e', key) + raise KeyError(key) + @staticmethod - def _raise_no_str_type(item): - raise TypeError(f"Key is of type {type(item)}; str expected") + def _raise_no_str_type(key): + raise TypeError(f"Key is of type {type(key)}; str expected") diff --git a/tests/golem/network/concent/test_received_handler.py b/tests/golem/network/concent/test_received_handler.py index d5f559a880..4b16fb809a 100644 --- a/tests/golem/network/concent/test_received_handler.py +++ b/tests/golem/network/concent/test_received_handler.py @@ -187,8 +187,7 @@ def tearDown(self): # Remove registered handlers del self.task_server gc.collect() - testutils.DatabaseFixture.tearDown(self) - testutils.TestWithClient.tearDown(self) + super().tearDown() class IsOursTest(TaskServerMessageHandlerTestBase): From 13d820bc6b10e452618836430663f0471dd8e52f Mon Sep 17 00:00:00 2001 From: maaktweluit <10008353+maaktweluit@users.noreply.github.com> Date: Fri, 21 Feb 2020 16:39:05 +0100 Subject: [PATCH 4/9] linter --- golem/apps/manager.py | 2 +- golem/apps/rpc.py | 2 ++ tests/golem/apps/test_app_manager.py | 2 -- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/golem/apps/manager.py b/golem/apps/manager.py index 2bfab08ae2..88bc3ced5b 100644 --- a/golem/apps/manager.py +++ b/golem/apps/manager.py @@ -20,7 +20,7 @@ def __init__(self, app_dir: Path, save_apps=True) -> None: self._app_dir = app_dir # Save build in apps, then load apps from path - built_in_apps = [] + built_in_apps: List[AppId] = [] if save_apps: built_in_apps = save_built_in_app_definitions(app_dir) for app_def in load_apps_from_dir(app_dir): diff --git a/golem/apps/rpc.py b/golem/apps/rpc.py index 8c74b5dfbc..d925e4ef8b 100644 --- a/golem/apps/rpc.py +++ b/golem/apps/rpc.py @@ -12,6 +12,8 @@ class ClientAppProvider: def __init__(self, client: 'Client'): + assert client.task_server is not None, \ + 'ClientAppProvider needs task_server' self.app_manager = client.task_server.app_manager @rpc_utils.expose('apps.list') diff --git a/tests/golem/apps/test_app_manager.py b/tests/golem/apps/test_app_manager.py index 7a34982d86..f89a1511ee 100644 --- a/tests/golem/apps/test_app_manager.py +++ b/tests/golem/apps/test_app_manager.py @@ -1,5 +1,3 @@ -from pathlib import Path - from golem.apps.manager import AppManager from golem.apps import ( AppDefinition, From 2cd79c4b1e3748f63b447f35c2c849096a89f563 Mon Sep 17 00:00:00 2001 From: maaktweluit <10008353+maaktweluit@users.noreply.github.com> Date: Mon, 24 Feb 2020 15:33:33 +0100 Subject: [PATCH 5/9] Store loaded app filename so it can delete it when requested --- golem/apps/__init__.py | 16 +++------------- golem/apps/manager.py | 11 +++++------ tests/golem/apps/test_app_manager.py | 7 ++++++- 3 files changed, 14 insertions(+), 20 deletions(-) diff --git a/golem/apps/__init__.py b/golem/apps/__init__.py index 40c0f629f7..879e354a34 100644 --- a/golem/apps/__init__.py +++ b/golem/apps/__init__.py @@ -1,7 +1,7 @@ import hashlib import logging from pathlib import Path -from typing import Dict, Any, Iterator, Type +from typing import Dict, Any, Iterator, Type, Tuple from dataclasses import dataclass, field from dataclasses_json import dataclass_json, config @@ -82,26 +82,16 @@ def load_app_from_json_file(json_file: Path) -> AppDefinition: raise ValueError(msg) -def load_apps_from_dir(app_dir: Path) -> Iterator[AppDefinition]: +def load_apps_from_dir(app_dir: Path) -> Iterator[Tuple[Path, AppDefinition]]: """ Read every file in the given directory and attempt to parse it. Ignore files which don't contain valid app definitions. """ for json_file in app_dir.iterdir(): try: - yield load_app_from_json_file(json_file) + yield (json_file, load_app_from_json_file(json_file)) except ValueError: continue -def delete_app_from_dir(app_dir, app_def: AppDefinition) -> bool: - filename = app_json_file_name(app_def) - file = app_dir / filename - if not file.exists(): - logger.warning('Can not delete app, file not found. file=%r', file) - return False - file.unlink() - return True - - def app_json_file_name(app_def: AppDefinition) -> str: filename = f"{app_def.name}_{app_def.version}_{app_def.id}.json" filename = sanitize_filename(filename, replacement_text="_") diff --git a/golem/apps/manager.py b/golem/apps/manager.py index 88bc3ced5b..cbefbefebf 100644 --- a/golem/apps/manager.py +++ b/golem/apps/manager.py @@ -2,9 +2,7 @@ from typing import Dict, List, Tuple from pathlib import Path -from golem.apps import ( - AppId, AppDefinition, load_apps_from_dir, delete_app_from_dir, -) +from golem.apps import AppId, AppDefinition, load_apps_from_dir from golem.apps.default import save_built_in_app_definitions from golem.model import AppConfiguration @@ -17,14 +15,15 @@ class AppManager: def __init__(self, app_dir: Path, save_apps=True) -> None: self._apps: Dict[AppId, AppDefinition] = {} self._state = AppStates() - self._app_dir = app_dir + self._app_file_names: Dict[AppId, Path] = dict() # Save build in apps, then load apps from path built_in_apps: List[AppId] = [] if save_apps: built_in_apps = save_built_in_app_definitions(app_dir) - for app_def in load_apps_from_dir(app_dir): + for (app_def_path, app_def) in load_apps_from_dir(app_dir): self.register_app(app_def) + self._app_file_names[app_def.id] = app_def_path for app_id in built_in_apps: self.set_enabled(app_id, True) @@ -78,8 +77,8 @@ def app(self, app_id: AppId) -> AppDefinition: def delete(self, app_id: AppId) -> bool: # Delete self._state from the database first del self._state[app_id] - delete_app_from_dir(self._app_dir, self._apps[app_id]) del self._apps[app_id] + self._app_file_names[app_id].unlink() return True diff --git a/tests/golem/apps/test_app_manager.py b/tests/golem/apps/test_app_manager.py index f89a1511ee..79dd61af26 100644 --- a/tests/golem/apps/test_app_manager.py +++ b/tests/golem/apps/test_app_manager.py @@ -1,3 +1,5 @@ +from mock import Mock + from golem.apps.manager import AppManager from golem.apps import ( AppDefinition, @@ -42,9 +44,12 @@ def test_re_register(self): def test_delete_app(self): self.app_manager.register_app(APP_DEF) + self.app_manager._app_file_names[APP_ID] = mocked_file = Mock() + mocked_file.unlink = Mock() self.assertEqual(self.app_manager.apps(), [(APP_ID, APP_DEF)]) self.app_manager.delete(APP_ID) self.assertEqual(self.app_manager.apps(), []) + mocked_file.unlink.assert_called_once_with() class TestSetEnabled(AppManagerTestBase): @@ -117,4 +122,4 @@ def test_register(self): app_file.write_text(APP_DEF.to_json(), encoding='utf-8') bogus_file.write_text('(╯°□°)╯︵ ┻━┻', encoding='utf-8') loaded_apps = list(load_apps_from_dir(self.new_path)) - self.assertEqual(loaded_apps, [APP_DEF]) + self.assertEqual(loaded_apps, [(app_file, APP_DEF)]) From 03b83db35f63d6bb20a4bdb9744c043d7663a1b8 Mon Sep 17 00:00:00 2001 From: maaktweluit <10008353+maaktweluit@users.noreply.github.com> Date: Tue, 25 Feb 2020 15:22:03 +0100 Subject: [PATCH 6/9] Raise instead of print errors --- golem/apps/rpc.py | 5 ++--- tests/golem/apps/test_app_rpc.py | 8 ++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/golem/apps/rpc.py b/golem/apps/rpc.py index d925e4ef8b..95848a6eaa 100644 --- a/golem/apps/rpc.py +++ b/golem/apps/rpc.py @@ -41,8 +41,7 @@ def apps_update(self, app_id, enabled): enabled, ) if not self.app_manager.registered(app_id): - logger.warning('App not found, not updated. app_id=%r', app_id) - return f"App not found, please check the app_id={app_id}" + raise Exception(f"App not found, please check the app_id={app_id}") self.app_manager.set_enabled(app_id, bool(enabled)) logger.info('Updated app. app_id=%r, enabled=%r', app_id, enabled) return "App state updated." @@ -51,6 +50,6 @@ def apps_update(self, app_id, enabled): def apps_delete(self, app_id): logger.debug('apps.delete called from rpc. app_id=%r', app_id) if not self.app_manager.delete(app_id): - return "Failed to delete app." + raise Exception(f"Failed to delete app. app_id={app_id}") logger.info('Deleted app. app_id=%r', app_id) return "App deleted with success." diff --git a/tests/golem/apps/test_app_rpc.py b/tests/golem/apps/test_app_rpc.py index 59f46bd837..98951aeb50 100644 --- a/tests/golem/apps/test_app_rpc.py +++ b/tests/golem/apps/test_app_rpc.py @@ -55,12 +55,12 @@ def test_update_not_registered(self): self._app_manger.registered.return_value = False # when - result = self._handler.apps_update(app_id, enabled) + with pytest.raises(Exception): + self._handler.apps_update(app_id, enabled) # then self._app_manger.registered.called_once_with(app_id) self._app_manger.set_enabled.assert_not_called() - assert result == f"App not found, please check the app_id={app_id}" def test_delete(self): # given @@ -79,8 +79,8 @@ def test_delete_failed(self): self._app_manger.delete.return_value = False # when - result = self._handler.apps_delete(app_id) + with pytest.raises(Exception): + self._handler.apps_delete(app_id) # then self._app_manger.delete.called_once_with(app_id) - assert result == 'Failed to delete app.' From d5130848a8053210ecb1f78099aec5499fa72e1d Mon Sep 17 00:00:00 2001 From: maaktweluit <10008353+maaktweluit@users.noreply.github.com> Date: Tue, 25 Feb 2020 15:42:11 +0100 Subject: [PATCH 7/9] Pass AppManager instead of Client to ProviderAppClient --- golem/apps/rpc.py | 8 +++----- golem/client.py | 4 +++- tests/golem/apps/test_app_rpc.py | 9 ++------- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/golem/apps/rpc.py b/golem/apps/rpc.py index 95848a6eaa..4427a3a2dc 100644 --- a/golem/apps/rpc.py +++ b/golem/apps/rpc.py @@ -5,16 +5,14 @@ if typing.TYPE_CHECKING: # pylint:disable=unused-import, ungrouped-imports - from golem.client import Client + from golem.apps.manager import AppManager logger = logging.getLogger(__name__) class ClientAppProvider: - def __init__(self, client: 'Client'): - assert client.task_server is not None, \ - 'ClientAppProvider needs task_server' - self.app_manager = client.task_server.app_manager + def __init__(self, app_manager: 'AppManager'): + self.app_manager = app_manager @rpc_utils.expose('apps.list') def apps_list(self): diff --git a/golem/client.py b/golem/client.py index 5df6319813..ac0084e943 100644 --- a/golem/client.py +++ b/golem/client.py @@ -267,7 +267,9 @@ def get_wamp_rpc_mapping(self): from golem.rpc.api import ethereum_ as api_ethereum from golem.task import rpc as task_rpc task_rpc_provider = task_rpc.ClientProvider(self) - app_rpc_provider = apps_rpc.ClientAppProvider(self) + app_rpc_provider = apps_rpc.ClientAppProvider( + self.task_server.app_manager + ) providers = ( self, concent_soft_switch, diff --git a/tests/golem/apps/test_app_rpc.py b/tests/golem/apps/test_app_rpc.py index 98951aeb50..b9bf3e462f 100644 --- a/tests/golem/apps/test_app_rpc.py +++ b/tests/golem/apps/test_app_rpc.py @@ -4,19 +4,14 @@ from golem.apps.rpc import ClientAppProvider from golem.apps.manager import AppManager from golem.apps.default import BlenderAppDefinition -from golem.client import Client from golem.testutils import pytest_database_fixture # noqa pylint: disable=unused-import -from golem.task.taskserver import TaskServer class TestClientAppProvider: @pytest.fixture(autouse=True) def setup_method(self): - self._client = Mock(spec=Client) - self._client.task_server = Mock(spec=TaskServer) - self._client.task_server.app_manager = Mock(spec=AppManager) - self._app_manger = self._client.task_server.app_manager - self._handler = ClientAppProvider(self._client) + self._app_manger = Mock(spec=AppManager) + self._handler = ClientAppProvider(self._app_manger) def test_list(self): # given From f00af9fba44e14f6b4dcb09d3ab67fc6bdecc098 Mon Sep 17 00:00:00 2001 From: maaktweluit <10008353+maaktweluit@users.noreply.github.com> Date: Tue, 25 Feb 2020 15:45:03 +0100 Subject: [PATCH 8/9] Import on lower level like the other RPC imports --- golem/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/golem/client.py b/golem/client.py index ac0084e943..e58ef94023 100644 --- a/golem/client.py +++ b/golem/client.py @@ -36,7 +36,6 @@ from golem import model from golem.appconfig import TASKARCHIVE_MAINTENANCE_INTERVAL, AppConfig from golem.apps.default import APPS -import golem.apps.rpc as apps_rpc from golem.clientconfigdescriptor import ConfigApprover, ClientConfigDescriptor from golem.core import variables from golem.core.common import ( @@ -266,6 +265,7 @@ def get_wamp_rpc_mapping(self): from golem.network.concent import soft_switch as concent_soft_switch from golem.rpc.api import ethereum_ as api_ethereum from golem.task import rpc as task_rpc + from golem.apps import rpc as apps_rpc task_rpc_provider = task_rpc.ClientProvider(self) app_rpc_provider = apps_rpc.ClientAppProvider( self.task_server.app_manager From 6ad3f6fe8a7d8085821ee6bcf6b9747af72d4561 Mon Sep 17 00:00:00 2001 From: maaktweluit <10008353+maaktweluit@users.noreply.github.com> Date: Wed, 26 Feb 2020 11:52:48 +0100 Subject: [PATCH 9/9] cleanup tupple breakdown and rename build_in_apps to new_apps --- golem/apps/manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/golem/apps/manager.py b/golem/apps/manager.py index cbefbefebf..8d0cea6840 100644 --- a/golem/apps/manager.py +++ b/golem/apps/manager.py @@ -18,13 +18,13 @@ def __init__(self, app_dir: Path, save_apps=True) -> None: self._app_file_names: Dict[AppId, Path] = dict() # Save build in apps, then load apps from path - built_in_apps: List[AppId] = [] + new_apps: List[AppId] = [] if save_apps: - built_in_apps = save_built_in_app_definitions(app_dir) - for (app_def_path, app_def) in load_apps_from_dir(app_dir): + new_apps = save_built_in_app_definitions(app_dir) + for app_def_path, app_def in load_apps_from_dir(app_dir): self.register_app(app_def) self._app_file_names[app_def.id] = app_def_path - for app_id in built_in_apps: + for app_id in new_apps: self.set_enabled(app_id, True) def registered(self, app_id) -> bool: