Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Manager class for applications using Task API #4657

Merged
merged 1 commit into from
Aug 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions golem/app_manager.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import json
import logging
from pathlib import Path
from typing import Any, Dict, List, Iterator

from dataclasses import dataclass, field
from dataclasses_json import config, dataclass_json
from marshmallow import fields as mm_fields

from golem.task.envmanager import EnvironmentManager

logger = logging.getLogger(__name__)


@dataclass_json
@dataclass
class AppDefinition:
name: str
requestor_env: str
Wiezzel marked this conversation as resolved.
Show resolved Hide resolved
requestor_prereq: Dict[str, Any] = field(metadata=config(
encoder=json.dumps,
decoder=json.loads,
mm_field=mm_fields.Dict(keys=mm_fields.Str())
))
max_benchmark_score: float
version: str = '0.0'
description: str = ''
author: str = ''
license: str = ''


def load_app_from_json_file(json_file: Path) -> AppDefinition:
""" Parse application definition from the given JSON file. Raise ValueError
if the given file doesn't contain a valid definition. """
try:
app_json = json_file.read_text(encoding='utf-8')
# pylint: disable=no-member
return AppDefinition.from_json(app_json) # type: ignore
# pylint: enable=no-member
except (OSError, ValueError, KeyError):
msg = f"Error parsing app definition from file '{json_file}'."
logger.exception(msg)
raise ValueError(msg)


def load_apps_from_dir(app_dir: Path) -> Iterator[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)
except ValueError:
continue


class AppManager:
""" Manager class for applications using Task API. """

def __init__(self, env_manager: EnvironmentManager) -> None:
self._env_manager = env_manager
self._apps: Dict[str, AppDefinition] = {}
self._state: Dict[str, bool] = {}

def register_app(self, app: AppDefinition) -> None:
""" Register an application in the manager. """
if app.name in self._apps:
raise ValueError(f"Application '{app.name}' already registered.")
self._apps[app.name] = app
self._state[app.name] = False
logger.info("Application '%s' registered.", app.name)

def enabled(self, app_name: str) -> bool:
""" Check if an application with the given name is registered in the
manager and enabled. """
return app_name in self._state and self._state[app_name]

def set_enabled(self, app_name: str, enabled: bool) -> None:
""" Enable or disable an application. Raise an error if the application
is not registered or the environment associated with the application
is not available. """
if app_name not in self._apps:
raise ValueError(f"Application '{app_name}' not registered.")
env_id = self._apps[app_name].requestor_env
if not self._env_manager.enabled(env_id):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EnvManager is only required for this single sanity check, I wonder if it's justified to make it a dependency. Especially that you can disable the environment just after enabling the app, thus introducing "invalid" state anyway. Maybe just performing these checks in the RequestedTaskManager when necessary is better.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reasoning is basically that if you don't allow to enable an app when the env is disabled you should disable all the apps when disabling the env as well - which we're not doing. It'd be circular dependency meaning that these classes should be merged, which doesn't make sense. Thus removing this check makes sense to me.

raise ValueError(f"Environment '{env_id}' not available.")
self._state[app_name] = enabled
logger.info(
"Application '%s' %s.",
app_name,
'enabled' if enabled else 'disabled')

def apps(self) -> List[AppDefinition]:
""" Get all registered apps. """
return list(self._apps.values())

def app(self, app_name: str) -> AppDefinition:
""" Get an app with given name (assuming it is registered). """
return self._apps[app_name]
2 changes: 2 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ constantly==15.1.0
crossbar==19.6.2
cryptography==2.7
cytoolz==0.9.0.1
dataclasses-json==0.3.0
dataclasses==0.6
distro==1.3.0
dnspython==1.15.0
Expand Down Expand Up @@ -54,6 +55,7 @@ ipaddress==1.0.19
Jinja2==2.10.1
lmdb==0.94
MarkupSafe==1.0
marshmallow==3.0.0rc6
miniupnpc==2.0.2
mistune==0.8.3
mock==2.0.0
Expand Down
2 changes: 2 additions & 0 deletions requirements_to-freeze.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ cffi==1.12.3
click==7.0
crossbar==19.6.2
dataclasses
dataclasses-json
distro
docker==3.5.0
enforce==0.3.4
Expand All @@ -23,6 +24,7 @@ golem_task_api==0.12.0
html2text==2018.1.9
humanize==0.5.1
incremental==17.5.0
marshmallow
miniupnpc<2.1,>=2.0
ndg-httpsclient
netifaces==0.10.4
Expand Down
97 changes: 97 additions & 0 deletions tests/golem/test_app_manager.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
from unittest import TestCase
from unittest.mock import Mock

from golem.app_manager import (
AppDefinition,
AppManager,
load_app_from_json_file,
load_apps_from_dir
)
from golem.task.envmanager import EnvironmentManager
from golem.testutils import TempDirFixture

APP_NAME = 'test_app'
APP_DEF = AppDefinition(
name=APP_NAME,
requestor_env='test_env',
requestor_prereq={
'key1': 'value',
'key2': [1, 2, 3]
},
max_benchmark_score=1.0
)


class AppManagerTestBase(TestCase):

def setUp(self):
super().setUp()
self.env_manager = Mock(spec_set=EnvironmentManager)
self.app_manager = AppManager(self.env_manager)


class TestRegisterApp(AppManagerTestBase):

def test_register_app(self):
self.app_manager.register_app(APP_DEF)
self.assertEqual(self.app_manager.apps(), [APP_DEF])
self.assertEqual(self.app_manager.app(APP_NAME), APP_DEF)
self.assertFalse(self.app_manager.enabled(APP_NAME))

def test_re_register(self):
self.app_manager.register_app(APP_DEF)
with self.assertRaises(ValueError):
self.app_manager.register_app(APP_DEF)


class TestSetEnabled(AppManagerTestBase):

def test_app_not_registered(self):
with self.assertRaises(ValueError):
self.app_manager.set_enabled(APP_NAME, True)

def test_env_not_enabled(self):
self.app_manager.register_app(APP_DEF)
self.env_manager.enabled.return_value = False
with self.assertRaises(ValueError):
self.app_manager.set_enabled(APP_NAME, True)

def test_enable_disable(self):
self.app_manager.register_app(APP_DEF)
self.env_manager.enabled.return_value = True
self.assertFalse(self.app_manager.enabled(APP_NAME))
self.app_manager.set_enabled(APP_NAME, True)
self.assertTrue(self.app_manager.enabled(APP_NAME))
self.app_manager.set_enabled(APP_NAME, False)
self.assertFalse(self.app_manager.enabled(APP_NAME))


class TestLoadAppFromJSONFile(TempDirFixture):

def test_ok(self):
json_file = self.new_path / 'test_app.json'
json_file.write_text(APP_DEF.to_json(), encoding='utf-8') # noqa pylint: disable=no-member
loaded_app = load_app_from_json_file(json_file)
self.assertEqual(loaded_app, APP_DEF)

def test_file_missing(self):
json_file = self.new_path / 'test_app.json'
with self.assertRaises(ValueError):
load_app_from_json_file(json_file)

def test_invalid_json(self):
json_file = self.new_path / 'test_app.json'
json_file.write_text('(╯°□°)╯︵ ┻━┻', encoding='utf-8')
with self.assertRaises(ValueError):
load_app_from_json_file(json_file)


class TestLoadAppsFromDir(TempDirFixture):

def test_register(self):
app_file = self.new_path / 'test_app.json'
bogus_file = self.new_path / 'bogus.json'
app_file.write_text(APP_DEF.to_json(), encoding='utf-8') # noqa pylint: disable=no-member
bogus_file.write_text('(╯°□°)╯︵ ┻━┻', encoding='utf-8')
loaded_apps = list(load_apps_from_dir(self.new_path))
self.assertEqual(loaded_apps, [APP_DEF])